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

New arch/capabilities #2508

Merged
merged 23 commits into from
May 6, 2019
Merged

New arch/capabilities #2508

merged 23 commits into from
May 6, 2019

Conversation

davigonz
Copy link
Contributor

@davigonz davigonz commented Apr 2, 2019

Implements #2495
Needs owncloud/android-library#246


Bugs & improvements

@davigonz davigonz changed the title New arch/capabilities New arch/capabilities [WIP] Apr 2, 2019
@davigonz davigonz mentioned this pull request Apr 2, 2019
21 tasks
@davigonz davigonz changed the base branch from master to new_arch/shares April 8, 2019 12:34
@davigonz davigonz force-pushed the new_arch/capabilities branch from dc9921a to 01977a3 Compare April 9, 2019 16:25
@davigonz davigonz force-pushed the new_arch/capabilities branch from 01977a3 to fb56ffd Compare April 9, 2019 16:54
@davigonz davigonz force-pushed the new_arch/shares branch 2 times, most recently from 98722e0 to f82ba8c Compare April 9, 2019 17:10
@davigonz davigonz requested review from jesmrec and abelgardep April 16, 2019 10:09
@davigonz davigonz changed the title New arch/capabilities [WIP] New arch/capabilities Apr 16, 2019
@abelgardep
Copy link
Contributor

@davigonz Some changes requested

@davigonz
Copy link
Contributor Author

@abelgardep changes applied

@davigonz
Copy link
Contributor Author

davigonz commented Apr 29, 2019

Code approved, ready to test @jesmrec , great job @abelgardep with the code review

@jesmrec
Copy link
Collaborator

jesmrec commented May 2, 2019

(1) [FIXED]

Capabilities are not being refresh after opening share view.

For example, if in UI you enable "Allow users to share via link", after opening share view, the "Public Share" section is visible.

Same happens with the password enforcing options.

@jesmrec
Copy link
Collaborator

jesmrec commented May 3, 2019

(2) [FIXED]

Check these steps:

  1. Disable feature "Allow users to share via link" in web dashboard
  2. Open share for any item -> Public sharing is hidden (OK)
  3. Go back to file list
  4. Enable feature "Allow users to share via link" in web dashboard and pull down to refresh
  5. Share again an item

Current: feature was enabled but public link section keeps hidden. By going back again to file list, and re-opening share, everything is correct. Seems that the first opening refresh capabilities, but do not apply changes in UI.

Nexus 6P v7

NOTE: the mentioned capability is the only affected. Any other capability related to public sharing is working as expected.

@davigonz
Copy link
Contributor Author

davigonz commented May 3, 2019

@jesmrec #2508 (comment) is ready to test but you will notice the public link section disappearing every time you open the shares view when Allow users to share via link option is disabled.

I do not like how it looks honestly, but since we are getting capabilities from network when opening the sharing view, there's nothing else we can do in terms of UI.

Maybe we can improve this behaviour during the implementation of capabilities when refreshing folders and just get capabilities from the database instead of server on the sharing view.

@michaelstingl
Copy link
Contributor

the public link section disappearing every time you open the shares view when Allow users to share via link option is disabled.

Could you post a GIF?

@davigonz
Copy link
Contributor Author

davigonz commented May 6, 2019

the public link section disappearing every time you open the shares view when Allow users to share via link option is disabled.

Could you post a GIF?

@michaelstingl
Copy link
Contributor

Ah, got it. Reverse it? Disabling items (empty for a few milliseconds), and enable only those that are valid?

@davigonz
Copy link
Contributor Author

davigonz commented May 6, 2019

Ah, got it. Reverse it? Disabling items (empty for a few milliseconds), and enable only those that are valid?

I kept it that way because I thought users will likely have the public shares enabled instead of disabled and the effect would not appear most of the times.

@jesmrec
Copy link
Collaborator

jesmrec commented May 6, 2019

yes, it is not usual that the capabilities are changing standly, they use to be static. Fix is correct.

Approved.

@davigonz davigonz merged commit e650c82 into new_arch/shares May 6, 2019
@delete-merged-branch delete-merged-branch bot deleted the new_arch/capabilities branch May 6, 2019 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants