don't delete change request or send email on correction failure

[Courey]

Description

We don't want to delete the change request or send any emails or kafka messages if the correction fails. When the promise to update is in the array of promises and gets called with promise.all, it fails after the change request is deleted and emails are sent. This means that no correction has persisted, but the request is gone.

What this branch does is calls the put alone. It then only calls promise.all if that put has succeeded. This means that if there is a failure to create the update, nothing will be sent and the correction request will remain. It may fail again if there is a bug that prevents it from succeeding, but it will hang around till we can fix the bug and try the approval again.

Testing

I tested this locally by:

1. adding a test case to the seed.json file. I used one from branch when a blank value is passed in, it should be undefined or it breaks … #3126 so I did not leave it in this branch since it would be a duplication. I used this test case because I knew it would fail without the corrections also included in that branch.
2. I tested the order that the functions were called within promise.all and found that the failure occurred after the approval was deleted and email sent.
3. I then made my corrections and retested the scenario.
4. The error occurred before anything else was called, so only the put was called and the error was thrown upon failure.
5. I was able to confirm the change request still exists after failure

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 6.51%. Comparing base (251cb12) to head (5a7459c).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
app/routes/circulars/circulars.server.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #3127   +/-   ##
=====================================
  Coverage   6.51%   6.51%           
=====================================
  Files        179     179           
  Lines       4423    4423           
  Branches     486     486           
=====================================
  Hits         288     288           
  Misses      4133    4133           
  Partials       2       2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[lpsinger]

What are the circumstances in which autoincrementVersion could fail? What problem does this solve?

[lpsinger]

What are the circumstances in which autoincrementVersion could fail? What problem does this solve?

@Courey, my guess is that you were responding to autoincrementVersion failing due to #3126, which was an internal bug, and not a normal and potentially recoverable failure (e.g. bad input from the user, or DynamoDB timing out). Normally we don't add error checking for faults that can only occur due to internal logical errors.

It is not clear to me that this is an improvement.

[Courey]

I did this because on failure, the request is deleted and the email is sent anyway. If it fails to put the version, it should not send an email or delete the request. By moving it out, if it fails, it never gets to the part that sends a success email or deletes the request. This means that if the moderator refreshes, they can try again. If it's not a temporary issue, they can also see what the error is and if there is a code fix required, the change request will still be there when a fix is released.