Skip to content
Snippets Groups Projects

Feature/dont pass profile to send_email_verification

Closed Feature/dont pass profile to send_email_verification
feature/dont_pass_profile_to_send_email_verification into develop
Closed Nicola Jordan requested to merge feature/dont_pass_profile_to_send_email_verification into develop

Created by: das-g

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
osmaxx/profile/views.py
18 18 class SendVerificationEmailMixin(object):
19 19 RATE_LIMIT_SECONDS = 30
20 20
21 def _send_email_verification(self, profile):
22 if cache.get(profile.associated_user.id):
21 def _send_email_verification(self):
  • Nicola Jordan
    Nicola Jordan @nicola.jordan
    Author Contributor

    Created by: hixi

    Do we ensure that the profile exists before calling this function? Before the caller had to pass the profile and take care of this. Maybe using an assert user.profile might remedy this issue?

  • Nicola Jordan
    Nicola Jordan @nicola.jordan
    Author Contributor

    Created by: das-g

    The current callers ensure this or at least assume it, otherwise they wouldn't have been able to pass the profile. But we can't really guarantee it for future callers or future revisions of the current callers.

    Would this be reason to keep the profile argument?

    An alternative refactoring would be to make _send_email_verification a public method of models.Profile.

  • Nicola Jordan
    Nicola Jordan @nicola.jordan
    Author Contributor

    Created by: hixi

    I think keeping the profile as parameter isn't a good idea, but having this method as public method on Profile sounds good to me.

  • Nicola Jordan
    Nicola Jordan @nicola.jordan
    Author Contributor

    Created by: das-g

    having this method as public method on Profile sounds good to me.

    PR for that approach, as an alternative to this one: #850

  • Nicola Jordan
    Nicola Jordan @nicola.jordan
    Author Contributor

    Created by: hixi

    I like that one much better, i puts readable names and clearer structure on the Profile module, which is the one that should be concerned with this, IMO. I accidentally already merged it, if you don't agree with the change merge, I'll revert it - otherwise, I think we can close this issue.

  • Nicola Jordan
    Nicola Jordan @nicola.jordan
    Author Contributor

    Created by: das-g

    Closing in favor of #850

    • Please register or sign in to reply