rubyforgood / share_christmas

Share Your Holiday Application
4 stars 3 forks source link

Edit Membership #110

Closed craig-riecke closed 8 years ago

craig-riecke commented 8 years ago

Implements #88.

craig-riecke commented 8 years ago

Note, there is one Rubocop offense that I don't understand well enough to fix:

-> rubocop
Inspecting 100 files
.................C..................................................................................

Offenses:

app/controllers/memberships_controller.rb:34:3: C: Assignment Branch Condition size for update is too high. [24.17/15]
  def update
  ^^^

100 files inspected, 1 offense detected
jaydorsey commented 8 years ago

There's a few ways you can fix it:

# rubocop:disable Metrics/AbcSize
def update

end
# rubocop:enable Metrics/AbcSize

I'd refactor if possible. Disable for that method if it's not possible. I wouldn't increase the size for the entire project unless you had a justification for it (other than "it keeps warning me" 😆 )

Could the check you have in the update method of the controller could be refactored so it's handled by the database validations & ActiveRecord validations?

craig-riecke commented 8 years ago

Ah yes ... as it turns out the duplicate email condition was already a validation that Devise added, so I just had to detect it! Fixed now.

craig-riecke commented 8 years ago

I got impatient and tests passed, so I merged this in.