More meaningfull message when a public authenticated share's password is wrong or has expired.

[StCyr]

Fixes#31952

Signed-off-by: Cyrille Bollu cyrpub@bollu.be

[PVince81]

👍 thanks!

[PVince81]

/rebase

[StCyr]

No idea why drone fails

[PVince81]

hmm, seems the test is expecting a specific message: https://github.com/nextcloud/server/blob/master/tests/acceptance/features/bootstrap/LoginPageContext.php#L69

I'd personally prefer to remove the message matching and rely only on classes.
If we do want to change the match, we'd need to separate the locator/selector because we use a different message now in the public link share page

@danxuliu is it common to expect specific messages like this in acceptance test ?

[danxuliu]

The locators are already separated, the one that fails is in PublicShareContext.php, not in LoginPageContext.php :-)

Using specific messages should be fine if they are stable enough, although I would also favour relaying instead on CSS classes and things like that. However, in this case the text needs to be used because the other elements are not specific enough; the message just has a warning class, so removing the text would make the locator a bit brittle.

Having said that, rather than adjusting the text in the locator I think it would be OK to add an explicit class to the message (like wrong-password or something like that) and use it instead of the text in the locator.

[PVince81]

@danxuliu thanks a lot for the info

@StCyr would you be able to fix the test ?

1. find the message in

   server/tests/acceptance/features/bootstrap/PublicShareContext.php

   Line 51 in 8951f06

   return Locator::forThe()->xpath("//*[@class = 'warning' and normalize-space() = 'The password is wrong. Try again.']")->

2. remove the message
3. add a CSS class to the message
4. change the locator to only use CSS (like it's done in the first part of https://github.com/nextcloud/server/blob/master/tests/acceptance/features/bootstrap/LoginPageContext.php#L69 )

To run acceptance tests locally:
% cd tests/acceptance
% ./run.sh features/app-files-sharing-link.feature:147

otherwise I can also take over 😄

[StCyr]

I think I did good @PVince81 ;-)

[danxuliu]

I think I did good @PVince81 ;-)

Happy to see an xpath expression replaced with a pure CSS one :-)

But... :-) Please keep in mind that the equivalent CSS expression would have been .warning.wrongPasswordMsg, without a space between the classes, so both classes refer to the same element; right now the locator expression is the element with class wrongPasswordMsg that is a descendent of an element with class warning, rather than the element with classes warning and wrongPasswordMsg. It works anyway because the div is inside a fieldset element with a warning class. In any case I have no strong preference for the previous expression, so feel free to amend it or keep it as is :-)

And if you already knew all that... then sorry for the noise ;-)

[PVince81]

let's move on. thanks a lot @StCyr and @danxuliu !

[PVince81]

/backport to stable24