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

[PAIR] Specify members to populate many members steps #572

Merged
merged 1 commit into from Dec 8, 2017

Conversation

luigi
Copy link
Contributor

@luigi luigi commented Dec 8, 2017

  • Previously, ManyMembersStepsController had to check if attributes
    were present, because there were times where only specific members
    populate a form, but when assigning attributes, it uses all members in
    application
  • No steps using ManyMembersStepsController have fields in the
    application, just on members, so no need to merge attributes
  • Remove some unused attributes from steps

[#153526809]

Signed-off-by: Paras Sanghavi paras@codeforamerica.org

 * Previously, ManyMembersStepsController had to check if attributes
 were present, because there were times where only specific members
 populate a form, but when assigning attributes, it uses all members in
 application
 * No steps using ManyMembersStepsController have fields in the
 application, just on members, so no need to merge attributes
 * Remove some unused attributes from steps

 [#153526809

Signed-off-by: Paras Sanghavi <paras@codeforamerica.org>
@luigi luigi changed the title Specify members to populate many members steps [PAIR] Specify members to populate many members steps Dec 8, 2017
@disambiguator disambiguator merged commit b7e4489 into master Dec 8, 2017
@disambiguator disambiguator deleted the refactor-many-members branch December 8, 2017 23:08
@disambiguator
Copy link
Contributor

@hartsick @jessieay @bengolder

Check out this PR! It's a small but significant change to the way we're handling many member steps. Less conditionals in shared code!

@hartsick
Copy link
Contributor

hartsick commented Dec 8, 2017

this is rad—

Copy link
Contributor

@jessieay jessieay left a comment

Choose a reason for hiding this comment

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

👍

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

4 participants