565 signup failure rollback

[JpadillaCoding]

Closes #565

What changes did you make?

• Added a rollback feature of removing the user from the database in case Cognito experienced a failure

  Rationale behind the changes?

• The database and Cognito would be out of sync in the case of Cognito experiencing a failure.
• Three approaches were possible:
  • Adding a deletion of the email if Cognito failed.
  • Changing the sequence to Cognito being completed first and updating the databases second, but this option still had the issue of having to rollback Cognito if the database request failed. I found it easier to reverse a database entry than reverse Cognito.
  • Embedding the Cognito request within the try and except blocks of the database entry so the built in session.rollback method could be used. I found that this made the code harder to read since it added more nesting of try and except blocks.
• I went with the first approach since it was the simplest and easiest to read. I made the deletion logic its own function for reusability.
• The cases for the Cognito Errors to rollback the database entry were kept for all cases except if the username already exists within Cognito. I checked each possible error and found this case to be the only one where an email should not get deleted since it signifies the user exists. Each case can be found here in the Errors section

  Testing done for these changes

• Manually tested Cognito failure by hard coding the try blocks password payload to an invalid password Password=body['password'] ---> Password='lol' This invoked the 'InvalidPasswordException' case and verified the email deletion worked.

What did you learn or can share that is new?

• Amazon documentation is vast and multiple links can have relevant information for your use case. It's good to ask around to make sure you did not miss anything

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied


Visuals after changes are applied


[JpadillaCoding]

@Joshua-Douglas I took your advice and used remove_user to avoid duplicate logic. To avoid the errors you mentioned and to also make the function more flexible for future uses, I made the deletion from cognito and the DB separate by specifying which to do on the parameters. I also set deleting from both as default as most use cases would call for the deletion of both. Let me know what you think of this approach.

As far as the test you provided, it worked fine as is and didn't need any modifications. The test failed without remove_user implemented on the case errors and passed after it was implemented.

Thanks!

[JpadillaCoding]

@tylerthome @paulespinosa @Joshua-Douglas Hey all. Thanks for your feedback! I've addressed the concerns pointed out 😄