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

Fit reg 1920 code #26276

Merged
merged 12 commits into from Dec 14, 2018
Merged

Fit reg 1920 code #26276

merged 12 commits into from Dec 14, 2018

Conversation

clareconstantine
Copy link

@clareconstantine clareconstantine commented Nov 30, 2018

(merge after #26265)

  • Flesh out FitWeekend1920Registration and FitWeekendRegistrationBase models
  • Update controllers, tests, and references to use the new classes.
  • Update frontend to not refer to 1819 in class names

@clareconstantine
Copy link
Author

@clareconstantine clareconstantine changed the base branch from fit-reg-1920-migration to staging-next December 3, 2018 21:41
photoRelease: [YES],
liabilityWaiver: [YES],
}.freeze
end
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of the content in here seems potentially year-specific; what's the long-term goal here?

Copy link
Author

Choose a reason for hiding this comment

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

I think much of what is in here will be relevant in future years, so I am erring on the side of starting with it in the base class and in future years we can move pieces that aren't being shared into year-specific classes, rather than pulling shared pieces back up into the parent from the year-specific classes.

return
end

super
Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't really a super class to invoke here. This should be removed. If there are fields that we'll always want, they should be here. Otherwise this should be a method that expects a derived class to override and throws an exception if it doesn't

Copy link
Author

Choose a reason for hiding this comment

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

now this just calls the super class (Pd::Form)'s method. Derived classes can add additional validations.

dashboard/app/models/pd/fit_weekend_registration_base.rb Outdated Show resolved Hide resolved
@sureshc sureshc closed this Dec 14, 2018
@sureshc sureshc changed the base branch from staging-next to staging December 14, 2018 19:16
@sureshc sureshc reopened this Dec 14, 2018
@clareconstantine clareconstantine merged commit 134993e into staging Dec 14, 2018
@clareconstantine clareconstantine deleted the fit-reg-1920-code branch December 15, 2018 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants