[Feature] Make review notes required for rejected

[andyrodrigues30]

Problem

Currently adding notes to a review is not mandatory. So verifiers can select a reason for rejection but leave the notes blank.

Proposal

We want verifiers to provide accurate and thoughtful notes to help improve the quality of guides. Making notes mandatory will help with this.

Throw error toast if the notes field is blank when submitting.

Area

app (frontend)

Confirmations

• I searched existing issues and this is not a duplicate.

[GamingDragonwastaken]

I'd like to take this one, though there's something worth raising before anyone spends time on it.

I think most of this already shipped. validateReview() in ReviewSidebar.tsx already blocks a rejection with empty notes and fires the error toast:

if (review.notes.length === 0) missing.push("a note");

git log -S puts that on main since 1 Aug (e8f7eae), so it predates this issue by a few weeks. Worth a maintainer sanity-checking that I'm looking at the right code path before this gets picked up by someone expecting to build it from scratch.

There is still a real gap, though. The check is length === 0, so a whitespace-only note (" ") passes validation and submits — a verifier can reject a guide with a note that's effectively blank, which defeats the point of the requirement. Trimming before the emptiness check closes that, and it's easy to cover with a unit test.

So: happy to take this scoped down to the trim fix plus a test, or happy for you to close it as already-implemented if you'd rather. Either is fine by me — I'd just rather flag it than open a PR that turns out to be a no-op.

[jpotter08]

I'd like to take this one, though there's something worth raising before anyone spends time on it.

I think most of this already shipped. validateReview() in ReviewSidebar.tsx already blocks a rejection with empty notes and fires the error toast:

if (review.notes.length === 0) missing.push("a note");
git log -S puts that on main since 1 Aug (e8f7eae), so it predates this issue by a few weeks. Worth a maintainer sanity-checking that I'm looking at the right code path before this gets picked up by someone expecting to build it from scratch.

There is still a real gap, though. The check is length === 0, so a whitespace-only note (" ") passes validation and submits — a verifier can reject a guide with a note that's effectively blank, which defeats the point of the requirement. Trimming before the emptiness check closes that, and it's easy to cover with a unit test.

So: happy to take this scoped down to the trim fix plus a test, or happy for you to close it as already-implemented if you'd rather. Either is fine by me — I'd just rather flag it than open a PR that turns out to be a no-op.

Thanks for flagging this. Yes, a basic check is already implemented, but if you could work on a better check as you mentioned that would be great. (Getting rid of whitespace / ensuring that there is at least one word.)