-
Notifications
You must be signed in to change notification settings - Fork 54
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
4311 - Missing display name fix #4363
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Owner names being saved in db as blank does not have much impact. As long as real_name is set to something blank (i.e: nil, " " empty string, etc), we will be using 'self[:login]' which is always true. At the end of the day, we save as nil if it is blank (which is the callback fix here). |
On the redirect part, this seems like a separate bug if there are sign up errors in new_trial that redirects to a different sign up form route. |
when :free_trial | ||
render :new_trial | ||
else | ||
render :new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If in case we add more registration_type, we can easily add it here
So there's one more change that's needed here to get to the desired behavior. The "Organization Name" field is not required for a trial sign up. It needs to be, so we actually force trials to have a display name (Organization Name = display name, just different label). |
Tried this, and it did not require a Organization Name. I was able to create a trial account without a display name. |
Please try restarting running server. |
e6ad82c
to
3d26875
Compare
3d26875
to
14a4535
Compare
This is ready @saracarl , let me know if there are any other change requests |
The fix works in 2 parts:
We update the callback
update_display_name
to set real_name and display_name tonil
in case the string value is blank. This will prevent new records with ' ' display_name values from being created.Second is we update the display_name override to detect [:display_name] presence, otherwise use login. This will help us so that we don't need to do a migration to fix bad data