Skip to content
This repository has been archived by the owner on Apr 7, 2024. It is now read-only.

Allow users to edit reviews #32

Closed
ankush opened this issue Oct 4, 2020 · 5 comments · Fixed by #42
Closed

Allow users to edit reviews #32

ankush opened this issue Oct 4, 2020 · 5 comments · Fixed by #42
Assignees

Comments

@ankush
Copy link
Collaborator

ankush commented Oct 4, 2020

Users should be able to edit or delete their reviews somehow.

We can achieve this by giving each review a specific page e.g. /review/id/ or as get parameter. Either way it would work.

Another way is to have per user reviews page. A page showing all reviews from logged in user.

Depends on #31

@ankush ankush added the mvp label Oct 4, 2020
@ankush ankush added this to the MVP milestone Oct 4, 2020
@ankush ankush removed the mvp label Oct 4, 2020
@ankush ankush removed this from the MVP milestone Oct 4, 2020
@ankush ankush added this to To do in REPL Reviews v1 Oct 7, 2020
@BlairCurrey
Copy link
Collaborator

We talked about a related issue that I think can be a subcomponent of this issue.

Each card needs a permalink (and an easy way to copy this link for sharing). We can change the reviews.html template to assign the review id to the card's html id. One of @amenat's proposed solutions for editing involved giving each review it's own page. If we do that, we can make the card a link to the review's page and then sharing that page will be straightfoward.

Alternatively, we can make a separate button or link that captures the card's position on the page via it's element id.

@BlairCurrey
Copy link
Collaborator

If we go the unique page per review route, should we utilize the same app.get("/reviews" ... route that we use for the full review page and review filtered by course page? We could use the same strategy that we use for filtering by course (check if the review id is specified and if so, use a WHERE clause to just get that one)

What's best practice? utilize the same route and make it flexible (I think this is the answer) or make more unique, single purpose routes?

@ankush
Copy link
Collaborator Author

ankush commented Oct 7, 2020

I'd suggest leaving /reviews page as is and adding a "singular" route /review/:id that links to individual review.

You'll get the id by using req.params.id in your function for that route.

@BlairCurrey BlairCurrey mentioned this issue Oct 9, 2020
2 tasks
@BlairCurrey
Copy link
Collaborator

Added the single page route as a subgoal #41 that I will tackle first

@BlairCurrey
Copy link
Collaborator

BlairCurrey commented Oct 9, 2020

A couple of broad ideas on this matter:

  • User shouldn't see an option to edit anywhere if they don't own the review
  • Check also needs to be performed on/before update query to ensure user owns review

What will the user experience be like? A user might have the review up in front of them when they want to edit. In that case, it would be nice to have a button right there. However, if a user is not currently at the review, there isn't a very direct way to reach it. You currently have to filter by the class or go through all the reviews. So I think a list of all reviews in the user profile is vital. Editing both places is ideal, but I think having it in the profile is the bare minimum.

Here is a rough outline of my plan.

  • Add list of user-owned reviews with 'edit' button in user profile
    • Add an edit button on the review itself (if the user owns it). Should make a new issue for this if not accomplished in this one.
  • make review update form with message flashing on top
  • make review update route

@BlairCurrey BlairCurrey self-assigned this Oct 9, 2020
@BlairCurrey BlairCurrey moved this from To do to In progress in REPL Reviews v1 Oct 9, 2020
@BlairCurrey BlairCurrey linked a pull request Oct 10, 2020 that will close this issue
@BlairCurrey BlairCurrey moved this from In progress to Review in progress in REPL Reviews v1 Oct 12, 2020
REPL Reviews v1 automation moved this from Review in progress to Done Oct 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants