Everything in the system treats a hidden row as absent — getPhotoFeedback drops it even for the guest's own feedback, and updatePhotoFeedbackStats does not count it. The per-viewer is_liked heart and my_color_label badge read the row without looking at is_hidden, so a like the photographer had hidden still showed as liked on a photo whose like_count was zero. Making those agree exposes why it had not been fixed: the duplicate check behind like/favorite toggling did not skip hidden rows either, so the now-empty heart, when clicked, found the hidden row and toggled it OFF — the click did nothing visible and the moderation was silently undone. Skipping hidden rows there makes the click create a fresh, visible row. Review found four more surfaces still treating a hidden row as present: the per-guest caps (an at-cap guest with one hidden met their own click with limit_reached), /my-feedback (which drives the Liked/Favorited/Rated chips in guest identity mode), getEventFeedbackSummary (disagreeing with the photo counters in the same response), and unhide (leaving two visible rows for one guest). The rating-clear and single-value delete scopes are visible-only now, so a follow-up mutation no longer destroys the admin's hidden record, and the unhide collapse is skipped when there is no stable identity to scope by — that fallback was 'guest_identifier IS NULL', i.e. other visitors' rows. Not taken: refusing to hide non-comment feedback, which the issue recommended. #839 and #1044 both ship hiding for reactions and colour labels with tests asserting a hidden one stops counting; only the admin UI's Hide button is comment-only. Merged with admin privileges: the author cannot self-approve.
This commit is contained in:
Paul Nothaft
@@ -147,7 +147,12 @@ class FeedbackService {
|
||||
*/
|
||||
async countGuestFeedback(eventId, feedbackType, guestId, guestIdentifier) {
|
||||
const query = db('photo_feedback')
|
||||
.where({ event_id: eventId, feedback_type: feedbackType });
|
||||
// Hidden rows do not count against the guest's cap (#1150). They are
|
||||
// absent everywhere else — the heart is empty, the tallies skip them,
|
||||
// and submitFeedback now treats one as room for a fresh row. Counting
|
||||
// them here would meet that fresh row with limit_reached and leave the
|
||||
// control dead until the guest un-likes something they can still see.
|
||||
.where({ event_id: eventId, feedback_type: feedbackType, is_hidden: false });
|
||||
if (guestId) {
|
||||
query.where('guest_id', guestId);
|
||||
} else {
|
||||
@@ -192,6 +197,13 @@ class FeedbackService {
|
||||
photo_id: photoId,
|
||||
event_id: eventId,
|
||||
feedback_type,
|
||||
// A hidden row is not there (#1150). Without this the guest saw an
|
||||
// empty heart — every read surface treats hidden as absent — and
|
||||
// clicking it found the hidden row and TOGGLED IT OFF, so the
|
||||
// click appeared to do nothing and it took two more to get back to
|
||||
// a filled heart. Skipping it makes the click create a fresh,
|
||||
// visible row, which is what the guest is asking for.
|
||||
is_hidden: false,
|
||||
});
|
||||
if (guest_id) {
|
||||
duplicateQuery.where('guest_id', guest_id);
|
||||
@@ -208,10 +220,15 @@ class FeedbackService {
|
||||
// into duplicate rows (same defense as the reaction path), and
|
||||
// clearing must not leave a stray duplicate in the average.
|
||||
if (isRatingClear) {
|
||||
// Visible rows only (#1150). A hidden row can now sit alongside
|
||||
// the guest's replacement, and it is the admin's moderation
|
||||
// record — clearing a rating must not destroy it, or there is
|
||||
// nothing left to review or unhide.
|
||||
const clearScope = db('photo_feedback').where({
|
||||
photo_id: photoId,
|
||||
event_id: eventId,
|
||||
feedback_type: 'rating',
|
||||
is_hidden: false,
|
||||
});
|
||||
if (guest_id) clearScope.where('guest_id', guest_id);
|
||||
else clearScope.where('guest_identifier', guestIdentifier);
|
||||
@@ -246,11 +263,16 @@ class FeedbackService {
|
||||
const singleValueColumn = SINGLE_VALUE_COLUMNS[feedback_type];
|
||||
if (singleValueColumn) {
|
||||
const submittedValue = feedback_type === 'reaction' ? reaction : color_label;
|
||||
// Visible rows only, same reason as the rating clear above: the
|
||||
// toggle-off and the duplicate collapse below both DELETE over
|
||||
// this scope, and a hidden original is the admin's record rather
|
||||
// than a racy duplicate of the guest's own.
|
||||
const singleValueScope = () => {
|
||||
const q = db('photo_feedback').where({
|
||||
photo_id: photoId,
|
||||
event_id: eventId,
|
||||
feedback_type,
|
||||
is_hidden: false,
|
||||
});
|
||||
if (guest_id) q.where('guest_id', guest_id);
|
||||
else q.where('guest_identifier', guestIdentifier);
|
||||
@@ -406,6 +428,11 @@ class FeedbackService {
|
||||
|
||||
const totalStats = await db('photo_feedback')
|
||||
.where('event_id', eventId)
|
||||
// Hidden rows do not count, the same rule the photo counters above
|
||||
// already apply — without this the two halves of THIS response
|
||||
// disagreed, and a hidden row preserved beside its replacement (#1150)
|
||||
// is counted twice.
|
||||
.where('is_hidden', false)
|
||||
.select(
|
||||
db.raw('COUNT(DISTINCT CASE WHEN feedback_type = ? THEN guest_identifier END) as unique_raters', ['rating']),
|
||||
db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_ratings', ['rating']),
|
||||
@@ -555,6 +582,11 @@ class FeedbackService {
|
||||
*/
|
||||
async moderateFeedback(feedbackId, action, adminId) {
|
||||
try {
|
||||
const target = await db('photo_feedback').where('id', feedbackId).first();
|
||||
if (!target) {
|
||||
throw new Error('Feedback not found');
|
||||
}
|
||||
|
||||
const updates = {
|
||||
updated_at: new Date()
|
||||
};
|
||||
@@ -569,17 +601,40 @@ class FeedbackService {
|
||||
updates.is_hidden = true;
|
||||
}
|
||||
|
||||
const feedback = await db('photo_feedback')
|
||||
.where('id', feedbackId)
|
||||
.first();
|
||||
|
||||
if (!feedback) {
|
||||
throw new Error('Feedback not found');
|
||||
}
|
||||
|
||||
const feedback = target;
|
||||
|
||||
await db('photo_feedback')
|
||||
.where('id', feedbackId)
|
||||
.update(updates);
|
||||
|
||||
// Unhiding can collide with a replacement (#1150). A hidden row reads as
|
||||
// absent, so the guest may well have re-added the same feedback in the
|
||||
// meantime; making the original visible again would leave TWO visible
|
||||
// rows for one guest on one photo — double-counted in the tallies, and
|
||||
// needing two toggles to clear because each one deletes a single row.
|
||||
//
|
||||
// Converge on the row the admin acted on, the same way the submit path
|
||||
// collapses racy duplicates. Comments are exempt: several from one guest
|
||||
// on one photo is normal.
|
||||
// Needs a stable identity to scope the collapse to ONE guest. With
|
||||
// neither id nor identifier the fallback degrades to
|
||||
// `guest_identifier IS NULL`, which is every identifier-less row on the
|
||||
// photo — other people's, deleted. Nothing to converge in that case, so
|
||||
// leave it alone.
|
||||
const collapseIdentity = feedback.guest_id || feedback.guest_identifier;
|
||||
if (updates.is_hidden === false && feedback.feedback_type !== 'comment' && collapseIdentity) {
|
||||
const superseded = db('photo_feedback')
|
||||
.where({
|
||||
photo_id: feedback.photo_id,
|
||||
event_id: feedback.event_id,
|
||||
feedback_type: feedback.feedback_type,
|
||||
is_hidden: false,
|
||||
})
|
||||
.whereNot('id', feedbackId);
|
||||
if (feedback.guest_id) superseded.where('guest_id', feedback.guest_id);
|
||||
else superseded.where('guest_identifier', feedback.guest_identifier);
|
||||
await superseded.delete();
|
||||
}
|
||||
|
||||
// Update photo stats if visibility changed
|
||||
await this.updatePhotoFeedbackStats(feedback.photo_id);
|
||||
|
||||
Reference in New Issue
Block a user