rubyforgood
1033 external form upload
🔗 Issue
https://github.com/rubyforgood/pet-rescue/issues/1033
✍️ Description
- Add a note for Google form support to description.
- Add a route and create a method to use form upload service to process the csv file
- adds test
📷 Screenshots/Demos
@kasugaijin There is an assertion that is commented. This is because it works perfectly when tested individually. But when using ./bin/rails test:all it fails. I don't know why the FormSubmission person_id is different from the user_id when tested with the mentioned command but it matches as it should when tested individually. I could not find where is it being affected.
Some thoughts https://www.loom.com/share/68033246a92b43a98e33fd72e466a625?sid=337cc0d5-0237-4e35-b6a3-d6b3328d3f72
i finish my thoughts here https://www.loom.com/share/7fc7094d4733480a8fa876abb7c0ec24?sid=6b6341d3-1bb8-42ed-8dcf-cd7ebf0e9f7e
I'm blocked, I cant make the factory for @person1 = create(:adopter, email: "[email protected]") to persist the person.
The person with the email "adoper1@..." is not being created. I've used :with_person to make changes to the factory but when debugging in the csv_service part of the code the person does not exist for some reason 🤯.
I'm blocked, I cant make the factory for
@person1 = create(:adopter, email: "[email protected]")to persist the person. The person with the email "adoper1@..." is not being created. I've used:with_personto make changes to the factory but when debugging in the csv_service part of the code the person does not exist for some reason 🤯.
Can you please push your latest changes up to this branch? I'd like to see the full picture before looking at why
@wandergithub I think I found the issues - see the video I'd suggest refactoring the CSV import logic first, too.
I think you can ignore my comment about changing the email in the setup and the csv...that should not be an issue with the updates I suggest.
https://www.loom.com/share/d462da8a6a534cd0b75b831fcc593786?sid=4cf176c6-89f8-408d-8aa1-289c46fb3c7f
@kasugaijin Thank you! I really needed that. I will implement the new service logic first to get it out of the way, and then check out everything else.
@kasugaijin Thank you! I really needed that. I will implement the new service logic first to get it out of the way, and then check out everything else.
I am glad it helped!
I am just leaving a note for myself here - we should make sure we fully test the refactor of the CSV import service i.e.
- it skips the row if the user exists and the timestamp matches that on the FormSubmisson
- it updates the existing form submission if it is 'empty' i.e. does not have any answers (no timestamp saved)
- it creates a new form submission and adds the form answers if there is no 'empty' form submission and the timestamp is different
@wandergithub You could add the above tests to this PR...but it has already changed a lot for you, so it might be better to not change the scope of this PR any further, and then tackle any of the missing above tests separately to keep things manageable (if you prefer).
I am just leaving a note for myself here - we should make sure we fully test the refactor of the CSV import service i.e.
- it skips the row if the user exists and the timestamp matches that on the FormSubmisson
- it updates the existing form submission if it is 'empty' i.e. does not have any answers (no timestamp saved)
- it creates a new form submission and adds the form answers if there is no 'empty' form submission and the timestamp is different
@wandergithub You could add the above tests to this PR...but it has already changed a lot for you, so it might be better to not change the scope of this PR any further, and then tackle any of the missing above tests separately to keep things manageable (if you prefer).
I added the last two since the first test already exists as the first one in csv_import_service_tests.rb. I think I addressed all concerns . I will be attentive to any comments.