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

Disabled_at logic: redirect to login path if user is disabled #9341

Conversation

jibees
Copy link
Contributor

@jibees jibees commented Jun 22, 2022

What? Why?

Closes #9336

What should we test?

  • As a customer, log in.
  • As a super admin, deactivate that user (in a separate session e.g. private mode of the browser).
  • As a customer, try to access your account (/account page).
  • See that you're redirected to /#/login

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@jibees jibees self-assigned this Jun 22, 2022
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good. I don't know if the other PR is solving the same thing? I'll leave that up to you.


context "as a disabled user" do
before do
user.disabled = '1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I just learned that we don't have to persist the user. Warden::Test::Helpers#login_as sets the user object in memory and unless we are doing other queries that need the database, we can use an in-memory user for authentication in specs!

That line here doesn't save to the database but we don't need to in this spec.

@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Aug 17, 2022

Hey @jibees ,

Before staging the current behavior I could observe is as follows:

Screencast.from.17-08-2022.20.13.28.webm

So, no snail, just a redirection to the homepage with the flash warning -> maybe because #9351 was merged in the meantime?

After staging this PR, the login appears:

Screencast.from.17-08-2022.20.24.21.webm

It kind of overlaps the with the flash warning, but I think this is ok. The user can either close the window and still see the flash warning; also, if a new login attempt is made with the same (disabled) credentials then the modal is clearly displayed.

Good to go 👍

@filipefurtad0 filipefurtad0 merged commit cedfa35 into openfoodfoundation:master Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error 500 (snail/slug) when deactivated user tries to access his/her account
3 participants