Johnwz123 / pe

0 stars 0 forks source link

Uploaded profile picture disappears after moving its location in file system #10

Open Johnwz123 opened 2 weeks ago

Johnwz123 commented 2 weeks ago

The upload works fine and the picture is correctly shown. However, what is stored in the data is the file path and not the picture itself. This causes the picture to disappear with the file of the profile picture is moved within your computer.

image.png

This is a significant bug since it would be very restrictive to have to keep the file path unchanged, especially as financial advisors with many files to track. Even if they keep all the pictures in a single folder, they are not able to rename the folder or shift it around.

Instead, a copy of the photos should be made and stored directly in the app folder instead to avoid this issue.

Steps to reproduce:

  1. Upload any profile picture from your local computer
  2. Check that it is correctly displayed
  3. Exit the app
  4. Move the file of your profile picture to a different location in your computer.
  5. Re-open the app. The picture will be removed.
nus-pe-bot commented 1 week ago

Team's Response

Thank you for your suggestion. We actually considered this during our design phase and actually implemented this initially (see our commit here), but we reverted to storing the file path rather than creating a copy due to a few reasons:

  1. Single point of failure. If we copy pictures into the app folder (data or some other folder we create), this implies that should the folder be corrupted/accidentally deleted, all profile pictures will be lost. The user will now have to re-upload everything from scratch. By not copying over to an app folder, it removes this concern. If the user chooses to voluntarily store it in a centralised location and upload from there, then the responsibility is no longer on us to keep that folder intact.

  2. Space savings. Copying pictures duplicates the space required to store an identical image, just in a folder closer to ours. Should the user upload an image with a massive file size, this can be detrimental to the person's storage space, especially if he/she repeatedly uploads big images.

  3. Emulating methods of big websites. It is common practice for websites to store images on the disk, and store the reference to the picture's location in the database, for performance and scalability reasons. This is more flexible and done by websites like Flickr (an image sharing platform) and Facebook. In our context, the "disk" will be the user's filesystem and the "database" will be our JSON file, which in fact stores the reference to the picture's location. We would like to take the opportunity to tap on a well-written elaboration here.

    Items for the Tester to Verify

    :question: Issue response

Team chose [response.Rejected]

Reason for disagreement: Thank you for sharing the considerations you had in making this design decision!

Nevertheless, this is still a significant functionality bug that is not addressed.

Users should not be expected to keep files in fixed locations or avoid renaming folders - this is an unreasonable constraint for a practical application and not something users should expect, so I would classify this as a functionality bug, rather than a feature flaw or documentation bug.

Furthermore, this behaviour or restriction is not mentioned in the User Guide so users unknowingly moving files or simply renaming folders would result in unpredictable loss of profile pictures.

image.png

The current design places an undue burden on users to maintain the file structure, which directly affects the usability of the app. Financial advisors, in particular, are likely to have to deal with a lot of files of different types and hence would likely frequently reorganise their files and folders to keep things tidy so this restriction would make your app less appealing in real-world scenarios, causing more than a "minor inconvenience", hence it should be more than low severity.

Nevertheless, I acknowledge that profile pictures are not critical information and missing profile pictures would not "cause major problems for users" so this does not warrant a high severity.

I believe minimally, rather than allowing profile pictures to simply disappear, a warning to users or a "File Not Found" error message should be provided to users to inform them of this behaviour if the app detects that the referenced file by the stored file path has been moved or deleted and prompt users to re-upload the missing file. Should this have been provided, I would classify this as a feature flaw instead of a functionality bug.

Alternatively, the app could offer users an option to choose whether to copy the image into the app folder or simply reference the file path during upload. This gives users flexibility based on their storage preferences.

Lastly, let me share some of my thoughts regarding your considerations:

  1. Single Point of Failure. Copying or saving pictures into an app folder managed by the app removes responsibility on the user to manage a centralised storage for all their profile pictures. This is common in software design. Many applications, such as photo management software and email clients, make copies of files to ensure seamless usage even if the original file is moved or deleted. Additionally, app data integrity can be safeguarded through automatic regular backups or warnings when a critical folder is about to be deleted. Moreover, losing all profile pictures at once is arguably better than gradual, unpredictable loss of individual profile pictures caused by users unknowingly moving or renaming files or folders, which is a more likely scenario in everyday use, since the mass loss of all profile pictures would alert users to an issue rather than silent gradual loss of individual profile pictures.

    While copying pictures into the app folder creates a single point of failure, this risk is manageable, such as through automatic regular backups and warnings as mentioned. This can also be supported by storing both the new file path to the copied picture in the app folder and original file path to the original uploaded picture file. This would allow one to serve as a fallback should an error occur to the other.

  2. Space savings. While space concerns are valid, they are not as critical given modern storage capacities. Users of this application, such as financial advisors, are likely using professional setups with sufficient storage space. If large image sizes are a concern, the app can include restrictions on file size limit or optimise uploaded images, compressing them to a smaller resolution, which would minimise space usage without compromising functionality. Storing file paths is only beneficial for saving space if the images are never moved, which is unrealistic for active users managing numerous files.

  3. Emulating methods of big websites. While it is true that large websites like Flickr and Facebook store references, this comparison is not fully applicable. These platforms host images on a centralized server as their "disk", ensuring that users cannot accidentally alter file paths or delete files. In fact, when you upload an image to an app like Facebook, what is stored in their database is not the file path to the image on your local file system. Rather, a copy of the uploaded image is made and stored in their centralized server (in this case for a smaller offline app could simply be an app folder) which is why uploading larger image files takes longer. What is stored in their database is thus the file path to the copy of the image on their centralized server.

    In contrast, your app depends on the user's local file system as the "disk", where changes to file paths are far more frequent and less controlled. Without safeguards to prevent this issue, your application does not fully emulate the robustness of such platforms.