search

codidact / qpixel

Q&A-based community knowledge-sharing software
https://codidact.com
GNU Affero General Public License v3.0
385 stars 69 forks source link

Fix not being able to edit privilege thresholds to 0 from the admin panel #1183

Closed Oaphi closed 1 year ago

Oaphi commented 1 year ago

This PR fixes a bug with privilege thresholds not being editable to 0. The script submitting the form used to parse the input value and short-circuit to null on any falsy value, resulting in 0 being interpreted as null.

cellio commented 1 year ago

Merge target isn't develop? What is the branch you want to merge to?

Oaphi commented 1 year ago

Merge target isn't develop? What is the branch you want to merge to?

Correct, @cellio, merge target is my other branch (for now) that fixes Docker error. The target is temporary to avoid unrelated commit from showing up (I'll add a note why [well, the note is not longer needed ^_^]).

Oaphi commented 1 year ago

Looks like you are correct about parseFloat handling null and undefined so we don't need the || ''. I've added a comment suggesting we could remove it, but mentioning it again here as it was a comment in a resolved conversation so I'm not sure if it will notify you.

@trichoplax Thanks for an extra mention! For the future, don't worry about me not getting the notification, though, I track resolved conversations until the PR is merged at least. Marked it as resolved as I've added the clarifying comment and want to double-check with the spec if I haven't forgotten anything [I think I haven't, but it never hurts to be sure] that will affect the removal of the short-circuit (thus making it a separate concern from what we've discussed).