Added is_private field to Review Response table

[lucas-meyer-7]

Hi @amritpurshotam , I think that you are right, and I made the DB changes that you recommended for the feature of specifying whether a response from a reviewer is private/public.

[amritpurshotam]

@lucas-meyer-7 Thanks this is a good start for this feature. Now to complete it there is still updating everywhere a review response is retrieved to filter what's returned depending on the type of user viewing them. You could probably get a good start by searching the usages of the ReviewResponse class and follow the code paths to the APIs that call them. We also would need to update and add tests for this feature.

P.S. Merge in this branch here to this one. It fixes the build for now then you can see what's breaking in the CI pipeline.

P.P.S I'm a bit at capacity for the next week or so my reviews have to be a bit shorter. Let me know though if this isn't enough to go on for the next few days.

[lucas-meyer-7]

Hi @amritpurshotam Thank you for your review. I appreciate the effort. About the "P.S. can we remove the commented code?", I am not sure exactly how the requests are handled (I'm very new to using reqparse) and I am still figuring it out. Will the is_private field be automatically generated for the reviewer to specify in the UI? I assume that we would still need to add some JS for all of that to work.

I have another question, not related to this feature but about applying for events [1] (see screenshots below for context). When I run the project locally, and apply to an event that I created locally, it throws an error [2] (Line 1095). This is because formResponse.formSpec is null and the request error is "No form exists with that Event ID" [3]. I'm not sure why this happens, but my best guess is that the error is thrown either because appropriate rows are not created in the database, or because I am running the project locally.

1: UI component - applying for a new event

2: Code in pages/applicationForm/components/ApplicationForm.js

3: Console lof of the received formResponse

[amritpurshotam]

@lucas-meyer-7

I am not sure exactly how the requests are handled (I'm very new to using reqparse) and I am still figuring it out.

Here's the docs. Have a look at the PostReviewResponseMixin specifically for the class you will need to modify for this request specifically. The code is quite self-explanatory. Pretty much just add a new argument for is_private.

Will the is_private field be automatically generated for the reviewer to specify in the UI? I assume that we would still need to add some JS for all of that to work.

This is correct, there will still need to be updates made to the front end.

And then the last part with your errors. I think that sounds like an error with the data. Did you insert the data manually? Have a look at these ERDs to see a higher level view of the tables and hopefully a better of the view of the app without having to click through every screen. There's a lot of configuration and access granted to the system based on the type of user you are and it's easy to get wrong (when doing so manually) and quite time consuming as well to do so through the front end. My advice for now though is not to get too caught up in the front-end and try to gain understanding from reading the API code, tests, and these tables.

[lucas-meyer-7]

@amritpurshotam Thank you for your answers and advice. I'll quickly make the changes required for this feature, excluding the front-end code required to make the is_private field appear to the applicant.

[amritpurshotam]

@lucas-meyer-7 Thanks this looks good so far. Some minor points though, can you change the branch you're merging into back to develop. Then merge in the fix/payment-test into this branch i.e.

git fetch
git checkout db_review_response_private_field
git merge fix/payment-test

I know this will clutter things a bit temporarily but once the fix/payment-test branch is merged into develop those changes will automatically disappear from your PR. This is just to get the build passing on your branch again and we can rely on all the automated checks it provides.

Otherwise going to leave this PR as is. We can only approve once the feature is complete. Feel free to branch off further from here though if you need the separation.

[lucas-meyer-7]

@amritpurshotam Ready for merge?

[amritpurshotam]

@lucas-meyer-7 Thanks this looks great with the build working again. Unfortunatelt thought there's still a bit more to do.

Since we've added the is_private column we still have to update the places that access review responses and filter based on the permissions of the user. Then the tests would need to be updated to reflect that filtering. And the front-end to specify whether a review response is private or not.

I know this is still a lot. Something you can do is branch off here to continue the above and open PRs that merge into this branch.