THREESCALE-12410: remove nil signup type - #4369
Open
jlledom wants to merge 3 commits into
Open
Conversation
Invited users previously received a nil signup_type because Invitation#make_user never set one. This was an implicit convention that relied on nil and :new_signup behaving identically everywhere except the observer email gate. This commit makes the type explicit by setting :new_signup in make_user, and guards the observer email with !user.invitation so invited users are not double-emailed (they already receive an invitation email and are activated directly by the controller). Part of THREESCALE-12410: simplifying signup types. Assisted-by: Claude Code
Now that invited users are explicitly set to :new_signup, nil is no longer a valid signup type in normal usage. Remove it from the by_user? coverage test to keep the test accurate. Assisted-by: Claude Code
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
jlledom
changed the title
nil signup type
RuboCop flagged the nested conditional as a style violation. Convert to an early return for consistency with the existing guard on line 7. Assisted-by: Claude Code
jlledom
marked this pull request as ready for review
August 7, 2026 11:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
In the effort to simplify the signup types handling, the initial idea was to remove all signup types and find a way to determine the kind of user via other ways on each point the signup type is referenced.
I abandoned that idea for the moment, I think the result wouldn't necessarily simplify the code. Instead I've been removing/merging signup types to leave only those that makes sense, IMO.
This PR removes the
nilsignup type and merges it with the:new_signuptype. All users that signed up via invitation will get the:new_signuptype from now on.The two types are pretty equivalent, the only place where an invited user diverges from what a self-signed-up does is during the signup process, on the user observer class.
The user observer adds an after commit callback that sends the signup notification:
Invited users must not receive this notification which makes no sense for them. This PR adds a guard to not send this notification for users having a not
nilassociation toInvitation.The PR also updates some tests. That should be it, everything else should behave the same as before.
For existing users in DB with signup type
nil, no migration path is needed, because they already behave exactly as:new_signupusers once the signup is completed.After this PR, there will be only three signup types left:
:new_signup: Users that signed up themselves, directly or via invitation.:minimal: Users created by db seed scripts.:created_by_provider: Users created by the provider, via API or UI.There's an excpetion: the
POST /admin/api/signupAPI endpoint also creates users as:minimal, but I didn't fix it to not break clients integrations.This is a summary of the differences between them:
:new_signup:minimal:created_by_provider* Unless invited
** Only when
approval_required?is false on the account plan AND the user has a password.Which issue(s) this PR fixes
https://redhat.atlassian.net/browse/THREESCALE-12410
Verification steps