Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build the albumMap even when the server returns a 403 #597

Merged
merged 1 commit into from
Mar 16, 2016

Conversation

oparoz
Copy link
Contributor

@oparoz oparoz commented Mar 15, 2016

Description

Album navigation breaks when the starting point is an empty Gallery and the user moves to the parent folder.
That's because the map we use for navigation is not built when the server returns an error status to a getFiles request.

Steps to reproduce

  1. Add pictures to a folder
  2. Create a sub-folder
  3. Enter the sub-folder
  4. Add a .nomedia file
  5. Switch to Gallery
  6. Press on the parent folder in the breadcrumb

Result before

TypeError: album.images is undefined  in galleryview.js

Result with this PR

No error

Tested on

  • Windows/Firefox
  • Windows/Chrome

Reviewers

@tahaalibra @viraj96 @imjalpreet @mbtamuli @rahulgoyal030

The albumMap is used for navigation, so it's important to build it whenever the user enters the app
@oparoz oparoz added the bug label Mar 15, 2016
@oparoz oparoz self-assigned this Mar 15, 2016
@oparoz oparoz added this to the 9.1-current milestone Mar 15, 2016
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @jancborchardt to be a potential reviewer

@virajparimi
Copy link
Contributor

I have tested it, and it works fine now. No error encountered by me. 👍

@mbtamuli
Copy link

I too found no errors. 👍 Works correctly.

@oparoz
Copy link
Contributor Author

oparoz commented Mar 15, 2016

Thanks for testing the fix. If I could ask you to keep using this branch to do normal operation to see if there are no side-effects, I would appreciate it as I didn't have time to thoroughly test this:

  • Switching between Files and Gallery always works
  • No problem on the public side
  • Uploading still works
  • Drag and dropping still works
  • etc.

The same effect will be achieved if I merge this, but a pre-run is always better ;)

@oparoz
Copy link
Contributor Author

oparoz commented Mar 15, 2016

Actually, even better would be for you guys to merge this PR with #593 in your dev environment. Just cherry pick the 2 commits into a new fork of master and use that when playing with Gallery.
The sooner those get merged, the sooner we can add some new features :)

@imjalpreet
Copy link
Member

@oparoz I have tested it by merging this with #593 and I have tried to test a few things and they seem to work well 👍

  • The test to check this PR.
  • Public Side is working
  • Switching between Files and Gallery
  • Uploading Files/Folders
  • Uploading using drag and drop
  • Transfer files/folders using drag/drop
  • Both sorts checked

I will continue to test this and get back to you if I find any error.

@oparoz
Copy link
Contributor Author

oparoz commented Mar 15, 2016

Perfect, thank you

oparoz added a commit that referenced this pull request Mar 16, 2016
Build the albumMap even when the server returns a 403
@oparoz oparoz merged commit 4aa247b into master Mar 16, 2016
@oparoz oparoz deleted the add-missing-albummap branch March 16, 2016 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants