Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor FormSubmission and FormAnswers #1232

Open
3 of 15 tasks
kasugaijin opened this issue Dec 9, 2024 · 4 comments
Open
3 of 15 tasks

Refactor FormSubmission and FormAnswers #1232

kasugaijin opened this issue Dec 9, 2024 · 4 comments
Assignees

Comments

@kasugaijin
Copy link
Collaborator

kasugaijin commented Dec 9, 2024

To do:

  • - Remove association to FormSubmission on AdopterApplication, AdopterApplication should belong to Person, add migrations
  • - update seed data to reflect new associations
  • - update the test suite and make fixes in code to reflect new associations
  • - Remove the creation of the FormSubmission on new user registration
  • - Update the CSV Import to always create a new FormSubmission if there is a new timestamp not yet imported (CSV Import: update to create new FormSubmission when there is a new timestamp for a given match on email #1243)
  • - Update the routes to scope form answers as their own resource (see below) and move the show action from the the adoption application reviews controller to the new controller, and the same for the show view - move to its new scope.
  • - Add policy class and tests
  • - update the use of this route (see the adoption applications page, we link to the /staff/adoption_application_reviews/:id in the Applicant column - so update this with the new link.
  • - update the use of this route (see the adoption applications page, we link to the /staff/adoption_application_reviews/:id in the Applicant column - so update this with the new link.
  • - update the link on the fosters page.
    image
  • - Fosters: If the Person model associated with this fosterer has a FormSubmission with FormAnswers present, we should link to the new resource created above. Otherwise, just link to the /staff/fosterers for now

To do in a follow up issue

  • - List historic form submissions on this route (and load Form answers with turbo frame)
  • - Add ability for adopter/fosterer to see their own form answers in their dashboard
  • - Update T&C to tell Orgs we don't want super sensitive data uploaded to our app
  • - Add an empty state to the show view for form answers. Currently, if there are no Form Answers available, it shows a blank page. We should instead show the following empty state 'Please ask this person to complete your questionnaire and then upload the CSV here', the 'here' text should link to /staff/external_form_upload

Background
We currently show a person's form answers at /staff/adoption_application_reviews/:id. We should instead treat them as their own resource scope e.g., /staff/form_answers/4.

We currently associate FormSubmission with AdopterApplication, but conceptully, this creates an unecessary dependence between the two. We can treat them as entirely separate concepts. The adoption application is the use showing interest. The ForSubmission (and its FormAnswers) represent a snapshot of that user's information. We don't need to connect the two for staff to carry out their application review successfully. All we have to do is show the FormSubmissions (including historic), and the date it was created. Then staff can decide if this is recent enough. We should also let adopters/fosterers see their form answers on the latest form submission so they can decide if they need to update anything.

@kasugaijin kasugaijin changed the title Form Answer Reviews: implement the new resource Form Answers: implement a new resource to allow staff to view a user's form answers Dec 9, 2024
@kasugaijin
Copy link
Collaborator Author

kasugaijin commented Dec 9, 2024

@jmilljr24 tagging you here - I think it makes sense to move form answer viewing to its own resource, rather than scoped to adotion application review, because orgs might give fosterers a (different) questionnaire as well. Our CSV import is role agnostic so this should work to import answers for fosterers as well, providing emails match. Right? The only thing we need to do is create the FormSubmission on the fosterer.

I went back and forth on the best solution here (creating FormSubmission in the CSV import versus creating it when we create the User and Person). Given that we have to have a FormSubmission to make an adoption applcation, we should create the FormSubmission when we create the Person and User. We currently don't do this in the Invite Fosterers flow, so should add this there.

The only issue I think our CSV importer might encounter is that if it finds a Person (emails match), it assumes there will be a latest_form_submission but what if this is a staff user, which is quite possible. I think it will fail. So, we should probably add a check there in the CSV importer to only proceed if the FormSubmission exists (adopter or fosterer) and return if it is nil (staff). Thoughts?

@kasugaijin kasugaijin added Ready Make a comment to get assigned. and removed Ready Make a comment to get assigned. labels Dec 9, 2024
@jmilljr24
Copy link
Collaborator

@jmilljr24 tagging you here - I think it makes sense to move form answer viewing to its own resource, rather than scoped to adotion application review, because orgs might give fosterers a (different) questionnaire as well. Our CSV import is role agnostic so this should work to import answers for fosterers as well, providing emails match. Right? The only thing we need to do is create the FormSubmission on the fosterer.

Correct. I would move to it's own resource as you've described.

With regard to different questionnaires, we would have an issue where a person is both an adopter and fosterer. If a adopter/fosterer had their adopter questionaire imported first, then had a forster questionaire imported, only their latest form submission would show. This would also mean if the person then went and applied to adopt, the foster questionaire would be associated with that application.

I feel like this is starting to get outside the initial intention of the csv upload. Something to think about though.

I went back and forth on the best solution here (creating FormSubmission in the CSV import versus creating it when we create the User and Person). Given that we have to have a FormSubmission to make an adoption applcation, we should create the FormSubmission when we create the Person and User. We currently don't do this in the Invite Fosterers flow, so should add this there.

That would be the easiest and keeps inline with what we have so that's my vote. I'm sure I've been told before( you can remind me next meeting) but I still don't quite understand the idea behind creating the form submission when the person(adopter currently) is created other than the fact we require the adoption application to have a form submission. If the form submission is empty, what is the point?

The only issue I think our CSV importer might encounter is that if it finds a Person (emails match), it assumes there will be a latest_form_submission but what if this is a staff user, which is quite possible. I think it will fail. So, we should probably add a check there in the CSV importer to only proceed if the FormSubmission exists (adopter or fosterer) and return if it is nil (staff). Thoughts?

The error will be rescued and the import will continue. The csv row and error will be displayed(once the pr is complete). I'm indifferent to adding a "fix" or just leaving it as is. It seems like a pretty rare edge case(maybe staff would be testing their form and used their email?). I'm sure there are a bunch of other obscure errors that may pop up during the import. I don't know if it makes sense to clutter up the service.

Not to add more complexity, but is there any reason to preclude staff from having questions/answers. If they have the data in the form, maybe it is important? This goes back to where/when the form submission should be created.

@kasugaijin
Copy link
Collaborator Author

kasugaijin commented Dec 9, 2024

With regard to different questionnaires, we would have an issue where a person is both an adopter and fosterer. If a adopter/fosterer had their adopter questionaire imported first, then had a forster questionaire imported, only their latest form submission would show. This would also mean if the person then went and applied to adopt, the foster questionaire would be associated with that application.

This is a good point. I was thinking that adopters and fosterers can be considered mutually exclusive from the org perspective. If they have worked with a fosterer, they should be comfortable enough processing that person on an adoption with the fosterer details. We can address this assumption if it causes issues in practice.

Not to add more complexity, but is there any reason to preclude staff from having questions/answers. If they have the data in the form, maybe it is important? This goes back to where/when the form submission should be created.

Yeah this had me thinking as well. We could make the AdopterApplication belong to the Person. Then, we create the FormSubmission during the CSV import, as and when is needed. This will enable all users to technically have a FormSubmission and FormAnswers, which seems fine to me. We do prevent staff from creating adopter applications in the policy permissions, but I don't think we need to change that for now.

@kasugaijin
Copy link
Collaborator Author

kasugaijin commented Dec 11, 2024

Discussion Notes:

  • Remove association to FormSubmission on AdopterApplication
  • AdopterApplication should belong to Person
  • Remove the creation of the FormSubmission on new user registration
  • Update the CSV Import to always create a new FormSubmission if there is a new timestamp not yet imported
  • Update the route to display form answers (see above)
  • List historic form submissions on this route (and load Form answers with turbo frame)
  • Add ability for adopter/fosterer to see their own form answers in their dashboard
  • Update T&C to tell Orgs we don't want super sensitive data uploaded to our app

@kasugaijin kasugaijin self-assigned this Dec 11, 2024
@kasugaijin kasugaijin changed the title Form Answers: implement a new resource to allow staff to view a user's form answers Refactor FormSubmission and FormAnswers Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants