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

When password protecting a previously created public link displays password policy error, the checkbox will be unchecked #9336

Closed
saw-jan opened this issue Jan 6, 2022 · 3 comments
Assignees
Labels
bug p3-medium Normal priority
Milestone

Comments

@saw-jan
Copy link
Member

saw-jan commented Jan 6, 2022

Steps to reproduce

  1. Enable Password Policy app on the server
  2. Set minimum password characters to 8 for public links (from Settings -> Admin -> Security)
  3. From desktop-client, open public link sharing dialog for a file and create a public link share
  4. Edit previously created public link
  5. Enable (check) Password protect and enter password 1234
  6. Click Set password
    (throws error The password is too short... and the checkbox will get unchecked but password filed can be edited)

Expected behavior

expected Password protect checkbox to be checked

Actual behavior

Password protect checkbox will get unchecked

pp01
pp02

Client configuration

Client version: 2.10.0rc3.6436
Operating system: win10x64

@saw-jan saw-jan added the bug label Jan 6, 2022
@TheOneRing TheOneRing added this to the 2.10 milestone Jan 10, 2022
@gabi18 gabi18 mentioned this issue Jan 10, 2022
51 tasks
@TheOneRing TheOneRing modified the milestones: 2.10, 2.10.1 Feb 14, 2022
@TheOneRing TheOneRing added the p3-medium Normal priority label Feb 14, 2022
@erikjv
Copy link
Collaborator

erikjv commented Feb 18, 2022

The password checkbox is unchecked, because the password has not been set. However, I do agree that this behaviour is a bit confusing. I think we should change it to either of the two following:

  • leave the checkbox checked, but disable the Close button when there is an error
  • move the line edit for the password and the error label into a modal dialog (with ok a
    nd cancel buttons), and when pressed Ok, will disable all controls until it has a server response, then make the UI like this:

Screenshot 2022-02-18 at 18 34 11

@TheOneRing What do you think?

@TheOneRing
Copy link
Member

Scenario 1sounds good.
One issue with the share dialog is also that it creates a share, and you can only modify it later.
So if you close the dialog, it will still exists and not be password protected.

I think you are right once the checkbox is selected, the close button must be disabled.
Same for expiration.

@fmoc fmoc self-assigned this Mar 11, 2022
@fmoc
Copy link
Contributor

fmoc commented Mar 11, 2022

I tested the dialog a bit. Seems like keeping the checkbox checked and the password field filled with the old value is what we do when creating new shares, too. The only difference there is that we do not (have to?) disable the close button there, since the link just hasn't been created.

I'm actually in favor with moving the editing controls into a secondary dialog. IMO this increases the usability, since with a modal dialog one cannot change to another entry in the list for instance while the other one's changes are sent to the server (IMO a "save"/"commit" button is missing anyway for editing, changes should be done in a "transactional" way). What do you think?

For a short-term fix, I'm evaluating the first option which will be easier to implement. But long-term, I'd like to rework the share dialog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug p3-medium Normal priority
Projects
None yet
Development

No branches or pull requests

4 participants