Conversation
|
Just to clarify - this is feedback for PENDING mods, to be given by verified devs/admins. This isn't for public use. |
|
@qimiko I've updated the comment with the new changes using the osu-like system you requested. |
|
I'll take a look at this today when I'm back from work, looks good at first glance |
alright thanks. @qimiko had already given me some improvements which i implemeted this morning. appreciate it! |
Fleeym
left a comment
Fleeym
left a comment
There was a problem hiding this comment.
Looks good overall, just some small nitpicks I personally had. We should also think about implementing something clientside for this
|
Alright, I applied the first 3 changes. I'm still not sure why "latest" bugs, but I'll look into it now. |
|
Latest is now fixed. Let me know if you need anything else! |
|
I've just added a delete endpoint. |
|
ok so i made like a million reviews i'm sorry 😭 but i left my thoughts on the current prq. you're free to question anything i say |
qimiko
left a comment
qimiko
left a comment
There was a problem hiding this comment.
very close 🙂 these specifically are my thoughts on what was added
| feedback TEXT COLLATE pg_catalog."default" NOT NULL, | ||
| decision BOOLEAN NOT NULL DEFAULT false, | ||
| type feedback_type NOT NULL, | ||
| dev bool NOT NULL DEFAULT false, |
There was a problem hiding this comment.
this could get a little confusing, considering that "this person is a developer of this mod" is not constant. i'd rather this check be done during the sql query (like the admin field) instead of stored in the table
| pub async fn delete_mod_feedback( | ||
| data: web::Data<AppData>, | ||
| path: web::Path<GetModFeedbackPath>, | ||
| payload: web::Json<DeleteModFeedbackPayload>, |
There was a problem hiding this comment.
instead of payload, put the id as part of the path:
/v1/mods/{id}/versions/{version}/feedback/{feedback_id}
this is more consistent with the rest of the api and also consistent with what a delete request is
| .or(Err(ApiError::TransactionError))?; | ||
|
|
||
| Ok(HttpResponse::NoContent()) | ||
| Ok(web::Json(ApiResponse { |
There was a problem hiding this comment.
i think this behavior is somewhat inconsistent considering the rest of the post endpoints don't do this, but i can see why you did it. i'll probably think about it further
There was a problem hiding this comment.
ok, i'll leave it as it is rn then
There was a problem hiding this comment.
on second thought, nocontent is probably the generally the best move
There was a problem hiding this comment.
how would you get the id? through the GET endpoint?
There was a problem hiding this comment.
ye, i think if the get feedback returns the author's feedback as well, then it makes it easy to associate feedback with the id on the client (so they can delete)
i wouldn't assume people are storing the id locally anyways
src/types/models/mod_feedback.rs
Outdated
| WHERE mf.mod_version_id = "# | ||
| ); | ||
| query_builder.push_bind(version.id); | ||
| if dev_only { |
There was a problem hiding this comment.
i was thinking about this a little more and i think it'd be better to allow returning the current user's feedback as well, like instead of dev_only you have filter_user: Option<i32> where None shows all feedback and having some value returns the feedback by that user id + the dev
so it doesn't just seem like your feedback was thrown into the void
3 participants
Feedback system
New Endpoints:
GET
/v1/mods/{id}/versions/{version}/feedback:Get mod feedback - only accessible to mod dev or admins
Example Response
{ "score": { "score" : 1, "positive": 2, "negative": 1 }, "mod_id": "sorkopiko.dailystreak", "mod_version": "1.1.0", "feedback": [ { "reviewer": { "id": 129, "display_name": "SorkoPiko" "admin": false, "dev": true }, "feedback_type": "Positive", "feedback": "Really cool mod! Might wanna fix a few visual bugs though, but overall amazing.", "decision": false } ] }POST
/v1/mods/{id}/versions/{version}/feedback:Add/update mod feedback - only accessible to verified devs and admins (mod devs can only leave notes)
Request Body Example
{ "feedback_type": "Positive", "feedback": "Really cool mod! Might wanna fix a few visual bugs though, but overall amazing." }DELETE
/v1/mods/{id}/versions/{version}/feedback:Delete feedback.
Request Body Example
{ "id": 4 }New Tables:
mod_feedbackColumns:
mod_version_id: Mod version ID, NOT mod version stringreviewer_id: Developer ID of reviewerfeedback: Feedback stringdecision: Whether this feedback decided the status of the modtype: Positive, Negative, Suggestion or NoteNotes: