[#11878] Fix AccountRequest migration script

[jayasting98]

Part of #11878

Outline of Solution

Checks if registeredAt is null. If it is, the status is set to APPROVED, otherwise it is set to REGISTERED.

Under the old logic, the admin adds an account request from the Google form responses into TEAMMATES only when the admin approves it. At this point, an account is not created yet, and registeredAt is null, but an email is sent to the instructor with a registration link; this corresponds to the APPROVED status in the new logic. When the instructor clicks on the registration link and registers then the account is created and registeredAt is non-null; this corresponds to the REGISTERED status in the new logic.

For more info, see:

• [#11878] Add status and comments to AccountRequest #12898 (comment)
• Account Request Form Upgrade #11878 (comment)
• Account Request Form Upgrade #11878 (comment)
• Account Request Form Upgrade #11878 (comment)

[EuniceSim142]

LGTM, thanks for the work!
Looking at the previous discussion, I agree with you that this might not be necessary. But if we don't plan on removing the migration script after v9-migration is complete, we can just merge this in to ensure correctness.

[ziqing26]

Thanks for the PR, but this change should not be needed as the migration script applies to datastore only. Since ARF project creates the new format of AccountReq in SQL, it should not affect the script.
cc @FergusMok @NicolasCwy Kindly help me double check on this 🙏

[NicolasCwy]

@ziqing26 @jayasting98 Yes I believe ZQ is right that this isn't needed. The migration scripts was meant to use as a one-off to migrate account requests(and other non-course entities) from Datastore to our SQL database, which should be completed next week.

I'm assuming the account request form feature will only be merged much later? So I think when the time comes you would need to create a migration SQL migration script which modifies the column values directly in the SQL database. Therefore you should not modify this script since the values don't mean anything till the ARF feature is merged

[jayasting98]

@ziqing26 @NicolasCwy Cool, thanks. Shall we just close this then?