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.

Sign up for GitHub

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

Jump to bottom

feat: #3713 show ToS checkbox if ToS error on login #3838

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: #3713 show ToS checkbox if ToS error on login #3838

wants to merge 1 commit into from

Conversation

florentcadot
Copy link
Contributor

One attempt to solve #3713

As written in , c2corg/v6_api#1542 should be merged first.

image

How to test:

  • For the backend checkout to the branch used in this PR: ToS acceptation v6_api#1542 and run the migrations
  • Launch the web app and try to login with any existing users (username: user590, mdp: c2c for instance), it should de facto fail as no users has accepted the ToS.
  • When it fail it should show the reason why it fail above the login form and if it's because the user didn't accept the ToS it should show a checkbox below the password field. If you check the box and press login it should successfully login

@github-actions github-actions bot added the enhancement New feature or request label Feb 11, 2024
@florentcadot florentcadot mentioned this pull request Feb 11, 2024
Open
@brunobesson brunobesson mentioned this pull request Feb 20, 2024
Open
brunobesson
brunobesson approved these changes Feb 20, 2024
View reviewed changes
Copy link
Member

@brunobesson brunobesson left a comment

Choose a reason for hiding this comment

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

Some minor comments, otherwise the PR looks really good. I haven't tested it yet however, for the same reasons explained in the PR for external resources in waypoints.

brunobesson
brunobesson requested changes Feb 20, 2024
View reviewed changes
Copy link
Member

@brunobesson brunobesson left a comment

Choose a reason for hiding this comment

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

Some minor comments, otherwise the PR looks really good. I haven't tested it yet however, for the same reasons explained in the PR for external resources in waypoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brunobesson brunobesson brunobesson approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@florentcadot @brunobesson