Review: Needs Fixing
Diff comments:
> === modified file 'src/plugin/folderlistmodel/dirmodel.cpp'
> --- src/plugin/folderlistmodel/dirmodel.cpp 2015-08-25 18:41:31 +0000
> +++ src/plugin/folderlistmodel/dirmodel.cpp 2015-09-07 21:20:53 +0000
> @@ -542,12 +542,20 @@
> }
>
> bool DirModel::allowAccess(const DirItemInfo &fi) const {
> - return allowAccess(fi.absoluteFilePath());
> + bool allowed = !mOnlyAllowedPaths; // !mOnlyAllowedPaths means any path
> is allowed
> + if (!allowed)
> + {
> + // for remote locations items are visible if them do not require
> authentication
> + allowed = mCurLocation->isRemote() ? !fi.needsAuthentication() :
> +
> isAllowedPath(fi.absoluteFilePath());
> + }
> + return allowed;
> }
>
> bool DirModel::allowAccess(const QString &absoluteFilePath) const {
> - return !mOnlyAllowedPaths || isAllowedPath(absoluteFilePath);
> -}
> + return !mOnlyAllowedPaths || mCurLocation->isRemote() ||
> isAllowedPath(absoluteFilePath);
This doesn't make sense to me. AllowAccess(absoluteFilePath) is supposed to say
if the given absolute file path is allowed access. Instead now it always allows
access if current location happens to be remote path, and doesn't in that case
take into account if the given path given in parameter is allowed access.
This function and allowAccess(const DirItemInfo &fi) should have the same
logic, they just receive their parameter in different representation. But it
must be said that after your refactorings it can be that either one of the
functions is not necessary anymore.
But if both exist they should either have same logic, preferably sharing the
code if possible, or the given function parameters and/or function name reflect
what's the real purpose here.
> +}// for remote locations access is allowed
> +
>
> void DirModel::onItemsAdded(const DirItemInfoList &newFiles)
> {
--
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-ui-01/+merge/270334
Your team Ubuntu File Manager Developers is subscribed to branch
lp:ubuntu-filemanager-app.
--
Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers
More help : https://help.launchpad.net/ListHelp
