-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Fix password enforce on public share links #12273
Conversation
You need to run the hanldebars script 😉 |
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
1cf8114
to
11b6c22
Compare
It's even worse, I forgot to add most of what I changed 🙈 😁 |
I tested it and it works almost 😉 If I have enabled the password policy app and use a password within the most used passwords, e.g. "helloworld" I get this: and for a short moment a error banner at the top "unable to create share link". The response contains the correct error message from the password policy app, but it seems like we don't read it and display it at the moment. |
Another small issue: It would be nice to put the focus directly on the input field so that the user can start directly typing. |
@schiessle Thanks!! |
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skjnldsv I tested it... works great! 👍
Most of the acceptance tests failures are unrelated: EDIT: passed on my setup, restarted the tests. |
@danxuliu can you take a quick look? I'm confused.
|
Yes, it is a hiccup. We may start to see this failure more often due to the redesigned UI for link shares; if that happens I will think how to fix it, but for now I hope that it is just an infrequent issue ;-) The problem is that clicking on the link modifies the link element to change its title from Copy link to Copied and back to Copy link; I have not checked it, but it is very likely that the element is also detached and attached again. The acceptance test gets the The tests are shielded against that due to using a wrapped element, but there is no support in the wrapper to get an attribute, so the pure Selenium object is used instead and due to that the problem can arise.
Good, you should not ;-) But in this case, feel free to merge :-) |
We should then use the input instead? :) Anyway, merging 😁 |
Fix #12272
New share id was different, therefore the popover check was not triggered.