-
Notifications
You must be signed in to change notification settings - Fork 0
Add ability to update puzzles from the admin table (Issue #30) #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -0,0 +1,11 @@ | |||
import { Controller } from "@hotwired/stimulus" | |||
|
|||
export default class extends Controller { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing the modal happens here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not reviewing your PR just a small comment; feel free to skip it, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on point of view, but the Puzzle model does not have at least one validation, meaning updating a record and removing all the content is valid. I'd say you need to add at least validates :question, presence: true
this way you should show an error when trying to save it blank.
I worked on it here #40, so this is out of the scope for this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved in case you don't want to extract the raw table, which i think it adds value because in the future we may forget about updating the raw in both places.
Great Job! TIL: how turbo-frames works (still learning tho)
Remove unnecessary dataset action check in modal close method. Update modal overlay to use click event for closing, ensuring consistent modal dismissal behavior.
|
||
def update | ||
@puzzle = Puzzle.find(params[:id]) | ||
if @puzzle.update(puzzle_params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[non-blocking/question]
Do we need to respond to json and html requests?
Is it possible to make an html/json request using the UI?
#30
This PR adds a modal to update the puzzles from the admin table. Before if there was a small mistake in a puzzle it was impossible to update it and it was usually rejected. Now the team that is looking over puzzles will be able to update them as well. I only allowed this to happen in the pending table so that we don't edit them in other tables once they have been approved, rejected, or archived. In my opinion it would be better for them to be moved back to the pending table if they need to be edited. Let me know if you disagree.
QA:
Click edit button on a puzzle and make sure you can edit all the fields, and that they are updated. make sure you can close to modal if you decide not to edit.