-
Notifications
You must be signed in to change notification settings - Fork 1.8k
enh: Gender as Dropdown in Attendee Form #3180
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
Conversation
|
@shreyanshdwivedi @uds5501 @prateekj117 Please Review |
|
@niranjan94 @CosmicCoder96 Please Review |
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.
@kushthedude LGTM, but now you will have to write a migration file too, which takes care of all the records which were already created. Send a PR on the server for that.
@CosmicCoder96 I don't think a migration file is needed, As in this PR https://github.com/fossasia/open-event-server/pull/5909/files when the gender field type was changes it was changed in whole custom_form schema, There we didn't explicitly define the relation of gender with any object like Speaker or Attendee. |
|
Database also stores the type for each custom form entry. Refactoring here
won’t refactor the dB.
…On Tue, 25 Jun 2019 at 11:15 PM, Kush Trivedi ***@***.***> wrote:
@kushthedude <https://github.com/kushthedude> LGTM, but now you will have
to write a migration file too, which takes care of all the records which
were already created. Send a PR on the server for that.
@CosmicCoder96 <https://github.com/CosmicCoder96> I don't think a
migration file is needed, As in this PR
https://github.com/fossasia/open-event-server/pull/5909/files when the
gender field type was changes it was changed in whole custom_form schema,
There we didn't explicitly define the relation of gender with any object
like Speaker or Attendee.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3180?email_source=notifications&email_token=AEDUDRPU4SBTWCPUFOVRYQ3P4JKTXA5CNFSM4H24J2Z2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYRBICA#issuecomment-505549832>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEDUDRJJD4ZHL3X7BXWNPVTP4JKTXANCNFSM4H24J2ZQ>
.
|
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.
LGTM
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.
LGTM. But this must be merged after the migration file on server is merged
|
@CosmicCoder96 Server PR is merged fossasia/open-event-server#6117 |
15a952d
|
@CosmicCoder96 @niranjan94 Please REview |
|
@CosmicCoder96 @niranjan94 Please REview |
Fixes #3176
Short description of what this resolves:
Attendee FormChecklist
developmentbranch.