Skip to content

Conversation

@kea-roy
Copy link
Member

@kea-roy kea-roy commented Sep 20, 2024

Summary

It can be really frustrating as a user when you make a small error and have to make an entirely new review from scratch.

With this feature, the edit reviews functionality streamlines the process and encourages users to refine their reviews, add photos, and update their reviews based on new experiences.

Changes Made

This pull request makes the following changes:

Test Plan

To test it thoroughly, I ran 15 "approval -> pending" tests, checking changes to each field separately. I made sure that the database was correctly updated and that the front-end user saw those changes reflected immediately in the UI.

01: Start State
01-start-state
02: Prefills Correctly
02-prefills
03: Made Edits
03-edits-made
04: Saved Correctly to Database and Retrieved Correctly in Admin Panel
04-reflected-in-admin-panel
05: After approval, correct information is shown on Apartment Page
05-approval-successful

I also tested the process to ensure that it works correctly with pending reviews on different pages, including the profile page and apartment page.
06: Start State of Pending Review on Profile Page
06-start-state-pending
07: Made Edits
07-edits-made
08: Updates Information Correctly and is displayed to the user
08-reflected-correctly

Notes

  • I would appreciate some feedback on idioms regarding the placement of helper functions. I placed convertReviewToFormData as the first constant in ReviewModel. From my past experience, typically constants that keep track of state are placed first and my choice seems to break that idiom. I chose to place it first since the const below it requires the helper function.
  • This Pull Request importantly adds key identifiers to ReviewComponents on every page, which means the components now refresh the information correctly and deleted components are shown as successfully deleted to the user.

@dti-github-bot
Copy link
Member

dti-github-bot commented Sep 20, 2024

[diff-counting] Significant lines: 321.

@kea-roy kea-roy marked this pull request as ready for review September 20, 2024 03:43
@kea-roy kea-roy changed the title [DRAFT] Implemented Edit Reviews Feature [Ready for Review] Implemented Edit Reviews Feature Sep 20, 2024
@kea-roy
Copy link
Member Author

kea-roy commented Sep 21, 2024

Changes Made

  1. Editing a review will not adjust the number of likes of the review.

Copy link
Contributor

@CasperL1218 CasperL1218 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Update Endpoint

  1. Great improvement on security checking with the additional layers of user authentication checking.
  2. The error messages are more descriptive and helpful.
  • backend/src/app.ts Line 106: While this is a client (frontend) post request that is closely associated with user authentication, I believe it will be better to return a 500 status code for catching a general (unexpected often) error. I am not fully sure about the endpoint development convention of this part, so I think it will be better to make sure with TPM for further confirmation.
  • backend/src/app.ts Line 95: I understand that the field checking conditions are inherited from previous code base, but I would like to make sure this means that we will not be accepting reviews where the reviewers simply wishes to provide a numerical/star/heart rating without textual comments? Is this currently reflected to the user that they must include something in the textual section of the review?

Frontend Review Display

  1. The changes regarding checking the [initialReview] state is accurate and well-done.
  2. I do not have the experience to comment on whether your convertReviewToFormData disrupts the convention, but I believe the design logic of the helper function is valid.

frontend/src/components/Review/Review.tsx

  1. The toast and the onSuccessfulEdit are well written.
  2. The replacement of static review fields deconstruction with a dynamic reviewData field is a good change.
  • An additional toast message for unsuccessful edit/update might also be helpful. This might be out of the scope of this PR, but we can consider this as part of future development.

Overall great work!

@kea-roy
Copy link
Member Author

kea-roy commented Sep 25, 2024

Changes Made

  1. Changed edit review endpoint return status numbers to match convention (4** to 5** for general server error)

@kea-roy kea-roy requested a review from CasperL1218 September 25, 2024 00:10
@kea-roy kea-roy dismissed CasperL1218’s stale review September 25, 2024 00:10

Resolved requested changes.

Copy link
Contributor

@ggsawatyanon ggsawatyanon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer your question: you can put the convertReviewToFormData definition under the other state tracking constants and then put the [review, dispatch] definition under that.

Overall, good work on this PR! One bug I've found is that when editing reviews that are not at the top of the pending reviews list, the toast only appears for 1 second and then disappears because the page is being reloaded before the toast's duration time completes.

- new optional prop to trigger the edit confirmation toast
- handle toasts on pages with reviews
CasperL1218
CasperL1218 previously approved these changes Oct 7, 2024
Copy link
Contributor

@CasperL1218 CasperL1218 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen.Recording.2024-10-06.at.21.22.12.mov
  • The network response error code has been changed to 500.
  • Tested and confirmed that the toast message will last for a reasonable duration when finish editing a pending review that is not at the top of the list.

Overall the issues are well resolved and the changes are effective.

@ggsawatyanon ggsawatyanon self-requested a review October 7, 2024 02:54
Copy link
Contributor

@ggsawatyanon ggsawatyanon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on making the toast message now not disappear. Unfortunately, I've found that the toast message doesn't appear when editing approved messages at all. Let me know if you need help with fixing this but this feature is super close to being approved, you got this!

- fixed when edited from approved reviews section on profile page
Copy link
Contributor

@ggsawatyanon ggsawatyanon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything works now, good job on fixing the requested changes!

@ggsawatyanon ggsawatyanon merged commit d8f7d57 into main Oct 8, 2024
4 checks passed
@ggsawatyanon ggsawatyanon deleted the edit-reviews branch October 8, 2024 22:51
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

Successfully merging this pull request may close these issues.

5 participants