Skip to content

Bug/noid/fix missing quickaccess favorite folder on add - #10888

Merged
rullzer merged 1 commit into
masterfrom
bug/noid/fix-missing-quickaccess-favorite-folder-on-add
Oct 24, 2018
Merged

Bug/noid/fix missing quickaccess favorite folder on add#10888
rullzer merged 1 commit into
masterfrom
bug/noid/fix-missing-quickaccess-favorite-folder-on-add

Conversation

@newhinton

Copy link
Copy Markdown
Contributor

When you mark a folder as favorite, it should be added to the quickaccessbar. This is not the case, because the route for checking if the newly marked element is a folder or a directory is missing. This PR readds the missing route.

@newhintonnewhinton added bug 3. to review Waiting for reviews labels Aug 27, 2018
@ChristophWurst

Copy link
Copy Markdown
Member

Hey, @newhinton,

thanks a lot for this fix. Could you please provide some reproduction steps for this? I don't fully understand your description, so I couldn't reproduce the problem.

Why do you mean by 'quickaccessbar'? The left sidebar in the files app?

@ChristophWurst

Copy link
Copy Markdown
Member

Oh, and commit 8d38f80 seems to be unrelated to this fix, correct?

@newhinton

Copy link
Copy Markdown
ContributorAuthor

yeah, 8d38f80 belongs to #10887 (actually, it IS #10887)
it must have sneeked in here

The quickaccessbar is the dropdown which gets added to the favorites-navigation-element in the filesapp navigation bar.

There are currently two ways do add an item to the bar:
Mark a folder as favorite with a right-klick;
Open details of said folder and klick the star in the details-bar;

Both should immediately add the folder to the quickaccess, which does not happen, because there is a route missing.

To make them show up, you need to reload the page

@juliusknorr
juliusknorr self-requested a review September 6, 2018 15:45
@juliusknorrjuliusknorr added this to the Nextcloud 15 milestone Sep 6, 2018
@skjnldsv
skjnldsvforce-pushed the bug/noid/fix-missing-quickaccess-favorite-folder-on-add branch from b9ed5d5 to 649c271CompareOctober 2, 2018 08:42
@skjnldsv

Copy link
Copy Markdown
Member

Rebased

@MorrisJobke

Copy link
Copy Markdown
Member

@skjnldsv Conflicting

@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 2, 2018
@skjnldsv
skjnldsvforce-pushed the bug/noid/fix-missing-quickaccess-favorite-folder-on-add branch from 649c271 to 5b94fc2CompareOctober 5, 2018 15:20
@skjnldsvskjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 5, 2018
@skjnldsv
skjnldsvforce-pushed the bug/noid/fix-missing-quickaccess-favorite-folder-on-add branch from 5b94fc2 to c8af5fbCompareOctober 5, 2018 15:21
@juliusknorr

Copy link
Copy Markdown
Member

@newhinton Can you rebase the changes and remove the unrelated 5d2ca94 commit? Otherwise it looks good from my side.

@newhinton
newhintonforce-pushed the bug/noid/fix-missing-quickaccess-favorite-folder-on-add branch 2 times, most recently from 5d8af24 to 52a66a3CompareOctober 10, 2018 11:13
…added to favorites
Signed-off-by: fnuesse <felix.nuesse@t-online.de>
@newhinton
newhintonforce-pushed the bug/noid/fix-missing-quickaccess-favorite-folder-on-add branch from 52a66a3 to 8dc4499CompareOctober 10, 2018 11:34
@newhinton

Copy link
Copy Markdown
ContributorAuthor

@juliushaertl should be fine now :)

@rullzer

Copy link
Copy Markdown
Member

mmm are we missing commits here?

@newhinton

Copy link
Copy Markdown
ContributorAuthor

@rullzer What do you mean? Is this fix not working?

@juliusknorr

juliusknorr commented Oct 24, 2018

Copy link
Copy Markdown
Member

@rullzer Doesn't seem so 😉

Tested and works as expected 👍

CI failure seems unrelated:

 Scenario: change email # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:136
Given I act as Jane # ActorContext::iActAs()
And I am logged in as the admin # LoginPageContext::iAmLoggedInAsTheAdmin()
And I open the User settings # SettingsMenuContext::iOpenTheUserSettings()
And I see that the list of users contains the user user0 # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
And I see that the mailAddress of user0 is "" # UsersSettingsContext::iSeeThatTheFieldOfUserIs()
When I set the mailAddress for user0 to "test@nextcloud.com" # UsersSettingsContext::iSetTheFieldForUserTo()
And I see that the mailAddress cell for user user0 is done loading # UsersSettingsContext::iSeeThatTheCellForUserIsDoneLoading()
Then I see that the mailAddress of user0 is "test@nextcloud.com" # UsersSettingsContext::iSeeThatTheFieldOfUserIs()
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-''
+'test@nextcloud.com'

@rullzer
rullzer merged commit b3a0d87 into masterOct 24, 2018
@rullzer
rullzer deleted the bug/noid/fix-missing-quickaccess-favorite-folder-on-add branch October 24, 2018 11:45
@MorrisJobke

Copy link
Copy Markdown
Member

@newhinton Mind to backport this to stable14?

@newhinton

Copy link
Copy Markdown
ContributorAuthor

@MorrisJobke I will try to do it on the weekend, i just create a pr with the fixes to stable14 right?

@juliusknorr

Copy link
Copy Markdown
Member

@newhinton Yes, we do backports by cherry picking the changes. The following git commands should do the job for this PR:

# checkout stable14 branch
git checkout stable14
# update branch to latest state
git pull
# create a new branch for the backport
git checkout stable14-10888
# cherry pick the commit from this pr into the backport branch
git cherry-pick 8dc449990dcd5a62e4280c1af9769582cf0a719a
# push your branch
git push

@MorrisJobke

Copy link
Copy Markdown
Member

@newhinton Do you need any further help for the backport? I plan to do the RC for the maintenance releases on Thursday so it should be back ported by then.

@newhinton

Copy link
Copy Markdown
ContributorAuthor

@MorrisJobke No, i just didn't found the time, but i will do it today, so there should be a PR ready tomorrow morning

@MorrisJobke

Copy link
Copy Markdown
Member

@MorrisJobke No, i just didn't found the time, but i will do it today, so there should be a PR ready tomorrow morning

Perfect. If you need help then we are here to do so :)

@newhinton

Copy link
Copy Markdown
ContributorAuthor

@MorrisJobke Done! :D

@MorrisJobke

Copy link
Copy Markdown
Member

@MorrisJobke Done! :D

Thanks 👍

Sign up for freeto 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

@newhinton@ChristophWurst@skjnldsv@MorrisJobke@juliusknorr@rullzer