JIRA-CHAL-29 #DONE Create a table in the database for voter registration data

[IshanBhatBhardwaj]

Overview

Created a table that will store all the users registration data, which will be encrypted. Also made a new srcipt called "npm run symlink. This will create a symlink between @/supabase/migrations/20240711063356_initial_schema.sql --> @/src/tests/supabase/migrations/20240711063356_initial_schema.sql

Now, when we update @/supabase/migrations/20240711063356_initial_schema.sql, @/src/tests/supabase/migrations/20240711063356_initial_schema.sql will also update.

Test Plan

All unit tests pass! Ran the symlink script on my system and it worked. Did not create any other testing method for this script.

Follow ups

Wondering if there are other ways to ensure that the script I wrote is safe and works. Perhaps another test file?

[dvorakjt]

Hi Ishan,

I really like what you've done so far here! Just a quick comment: the unit tests are failing when running in the Github actions workflow because they can't find the migration file. Based on some brief reading, it seems that Github actions may not be able to process symlinks. This means that you would need to actually copy the migration file(s) into the __tests__/supabase folder before the command npm run test:ci runs (you could create another node script to do this and add it to the npm run test:ci command). Please address this first and ensure that the tests are passing when executed by our Github actions workflows and then I'll be able to review the code further.

Thank you!

[dvorakjt]

Hi Ishan,

This looks great. I was able to create the symlink on my computer with your script, although I had to run the command prompt as an administrator to do so. I verified that that the two files were indeed linked and that I could run the test database after creating the symlink. Really great work, impressive!

Now, there are few follow-ups:

• It looks like the idNumber field is missing from the table. Please double check that you
  have added fields for all of the parameters of the /registertovote endpoint of the USVotes API
• There are now a couple related script files in the root directory. Maybe we should create a directory called scripts for these?
• I think the name of the symlink script inside package.json could be slightly more descriptive
• I think you can remove the named script to copy the migration file and just call it directly with node in the test:ci script since that's probably the only time it will be called
• Maybe add some empty lines to the output of your script so that it is more legible (maybe the output could be made more concise, too?)
• If you want to write tests for this script, I support it
• Since our workflow will be different after the addition of the symlink script, this should be documented in the README. Please add a note for windows users that they may need to run the command prompt as an administrator to run the script
• When I checked out into the pr, the migration file in the tests/supabase folder was included, but it did not create an actual symlink. Do we remove and gitignore this file and encourage folks to run the symlink script?

This is very cool and solves this problem nicely!

[IshanBhatBhardwaj]

Fixed my script bug! As the previous PR comment noted, I moved my scripts into a script folder. However, I forgot to change my paths for creating the symlink. Once I changed the paths, the bug was fixed.

Unfortunately, now all the checks are failing. I am not too sure why these checks are failing, but will look more into it. I also noticed I forgot to go into the sql file and add the idNumber again. Will definitely go back and do that soon!

Lastly, I do not think we need to remove and gitignore the sql file. When running the create-symlink-supabse-to-src command, it can detect a file, delete it, and replace it with the symlink.

Overall, should have the task done by tomorrow at the latest! Just need to figure out why the checks are failing.

[dvorakjt]

Hey Ishan, I think I see why the scripts are failing. I'll get back to you in a minute.

[dvorakjt]

Yeah, actually an easy fix. Have a look at the .github/workflows/run-unit-tests.yml file. You'll see that npm run supabase-test:start is called before npm run test:ci. This is my mistake, the command to copy the migration file actually should be its own command and should be run inside this workflow before starting supabase. I think this should take care of it as the error I see in the workflow run is that the migration file could not be found.

Additionally, since you are working on this, I'm wondering if you can modify your code to link (and copy) the migrations directory instead of the individual file?

[IshanBhatBhardwaj]

When looking at the details, I saw that my copy migration file script failed. I think its due to the paths:

path: '/home/runner/work/8by8-challenge/8by8-challenge/supabase/migrations/20240711063356_initial_schema.sql',
dest: '/home/runner/work/8by8-challenge/8by8-challenge/src/tests/supabase/migrations/20240711063356_initial_schema.sql'

Not sure why 8by8-challenge occurs twice. I will look more into it and see if I can figure it out.

Also, yes I beleive I can create a symlink between folders. However, it gets tricky when the folder already exists. I have to first ensure that all the content within the folder is already deleted. Will do some more research and try to get it implemented.

[dvorakjt]

I see, sounds good. I think the copy script will still need to be executed in the workflow file prior to the supabase-test:start script regardless as Supabase will need to access the migration file before starting.

EDIT: while I'm thinking about it, what is the reason that it has to relink the folder if it exists? I think it could just throw an error if the folder exists, instructing the developer to remove it in order to create the link. If we then remove the tests/supabase/migrations folder and subsequently add it to the gitignore, developers should only ever need to run your script once anyway

[IshanBhatBhardwaj]

Quick update, still trying to get the checks to pass on git. Also was wondering if there Is a way to run the github checks without pushing my code everytime?

Yea! What you said makes sense. I originally thought that developers would already have the tests/supabase/migrations folder, so I wanted to make a script that would be easier for other developers to run in the future. But if we add the directory to the git ignore, then it shouldn't be much of a problem. It would also make the script a lot easier.

[dvorakjt]

Great! Yes, if you click on the actions tab, you can find the job and re-run it. In this case, it looks like the copy script is still failing for some reason (although it's just logging the error without actually failing). This appears to be causing the tables not to be created correctly, which is then causing the unit tests that talk to Supabase to fail

[dvorakjt]

Looks good! Only two things I see:

• There have been a few commits made to the 8by8 development branch, please fetch these changes so your repo is up-to-date
• In the test:ci script, you probably don't need to copy the migrations directory again since this is now handled by the run-unit-tests.yml file

[dvorakjt]

Edit: I triple checked the code, and found that you are missing another required field from the /registertovote endpoint of the US Votes API: https://github.com/8by8-org/usvotes. I apologize for not catching this sooner. Please review the documentation for the /registertovote endpoint and ensure that every required field is present in your table.

[IshanBhatBhardwaj]

Sorry about that! I saw that I was missing the state parameter and just fixed it. I did name it state_ because it seems like state was a reserved keyword. If there is a more appropriate method on naming the varaible, let me know and I can fix that if needed! Thanks again!

[dvorakjt]

Approved! There will be a few small items to continue to iterate on, which I will go over in the Jira story for creating the register to vote endpoint.