Skip to content
This repository was archived by the owner on Jan 19, 2022. It is now read-only.

Refactor invite#263

Closed
yeukhon wants to merge 5 commits intomozilla:masterfrom
yeukhon:refactor-invite
Closed

Refactor invite#263
yeukhon wants to merge 5 commits intomozilla:masterfrom
yeukhon:refactor-invite

Conversation

@yeukhon
Copy link
Contributor

@yeukhon yeukhon commented Sep 12, 2013

Finally got a few hours tonight to finish this! Refactoring went pretty smoothly. Manual testing is time consuming.

  • refactored the invites/control into resend, decline and accept apis
  • refactored common code into load_user_and_invite and invite_has_expird
  • extend invite lifetime when resend instead of issuing a new id

This requires mozilla/minion-frontend#140 to be pulled in.

When a user logs in with an existing account in Minion we remove
the account that was created for the invited user. We update all
the group associations to the login from the original invite
account.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is an appropriate solution. First of all, the error reporting does not work. You call jsonify to create an error rrsponse but it is never returned. If you have to return it then the contextmanager would have to return a tuple with (error, invitation, user) which does not seem to be the right thing. Context managers were not really intended for this kind of code.

I would prefer to simply see load_invite and validate_invite functions that return an error if the invite is bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm the nice thing about this contextmanager "hack" is it allows us to do enter and exit. I probably could do decorator @ but i have to re-read both documents which is resource wasteful. I do this initially because I kinda want to move away from the C-like style where we do

user, invitation = func(...)

I think we can do that. I was hoping to find a api-level enter, exit scope. Maybe thats what class api is for :)

Let;s remove context manager then.

…me of invite instead of sending a new id.
@cknowles-admin cknowles-admin added the ARCHIVED - http://mzl.la/ghe-archive CLOSED at time of archiving label Jan 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ARCHIVED - http://mzl.la/ghe-archive CLOSED at time of archiving

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants