Conversation
|
The commit 146957c works to retrieve all of the checked-out items but will require updating the test database. It's likely that we will need to run a total count similar to summary.js/within the backend calls in order to retrieve total numbers and counts. |
|
The current plan is to open another PR that will handle the frontend changes necessary to link the Summary.js with the backend. There we will be able to calculate the total number of checkouts and unique students necessary to fulfill task 3 and 4. |
parthpandey1
left a comment
There was a problem hiding this comment.
@jayrevolinskyjr This looks good but could you add authentication to the 3 endpoints ? I see that ensureAuthenticated has been passed as a parameter but you'd have to add an if-else statement to ensure authentication. Something like if (!req.isAuthenticated()) { res.status(401).send("Unauthenticated"); return; } else{ //rest of your code }
You can lookup donor.js file for reference.
@parthpandey1 I have addressed the changes by adding if-else authentication checks on all of the /items routes. The screenshot shows an example of the routes behaving properly when an authenticated or unauthenticated request is made on a given route. |
parthpandey1
left a comment
There was a problem hiding this comment.
Looks good! I went ahead and tested the endpoints using postman with authentication and it works flawlessly
reembot
left a comment
There was a problem hiding this comment.
/total_donation and /total_checkouts have been tested and look good. Authentication messaging edits also tested and looks good.
For /unique_checkouts, we'll need richer dummy data with multiple person_ids to ensure this is working properly (currently only a single person_id is being used for INSERTs)
@jayrevolinskyjr and I attempted to create additional inserts with different person_ids but with no luck. Will require further prodding.
@reembot I updated the dummy_data with an additional person and added an explicit checkout option that was missing. @parthpandey1 It appears that the Feed tests are failing since they rely on dummy data. I will investigate further for fixing the Feed tests related to Issue #208. It's likely Feed.test will need to be updated before any more merges can be done. Update 05/13/2023: Feed tests were fixed with #243, rebased with main to pass tests and coverage. |

This is the PR related to Issue #223. Current plan is to implement backend before the frontend is designed/started.