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

Signup: unify instant_confirmation and request_confirm #1139

Closed
wants to merge 4 commits into from

Conversation

felixrindt
Copy link
Member

In an effort to write smaller PRs, here's the refactored simple signup method as discussed in the design blog.

@felixrindt felixrindt added the [C] refactoring refactors code without changing functionality label Nov 11, 2023
@coveralls
Copy link

coveralls commented Nov 11, 2023

Coverage Status

coverage: 88.035% (-0.2%) from 88.211%
when pulling 38be5d0 on unify-simple-signup
into a985d49 on main.

@felixrindt felixrindt changed the title unify instant_confirmation and request_confirm Signup: unify instant_confirmation and request_confirm Nov 11, 2023
AutomaticConfirmationMixin, QualificationMinMaxBaseSignupMethod
):
slug = "simple_qualification_required"
verbose_name = _("Signup where a qualification can be required")
Copy link
Member Author

Choose a reason for hiding this comment

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

@jeriox what do you think of this name? I think this style of phrasing would allow users to better differentiate between the couple/no sign up methods as well as between the actual signup methods. It also shows that the qualification requirement is optional

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I'd like to see all of the names next to each other to judge that. My first thought was that starting the name with "signup" is a bit redundant since the field is already labeled with signup method.
I still like the idea of using the planners POV since they need to grasp the concept...

Copy link
Member Author

Choose a reason for hiding this comment

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

We can change the label on the form field though. Or do some custom templating entirely.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should review the names when section-based and complex is finished.

@felixrindt felixrindt closed this Jun 11, 2024
@jeriox
Copy link
Contributor

jeriox commented Jun 11, 2024

replaced by new flow/structure concept in #1202

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C] refactoring refactors code without changing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants