Inadvertent flood of emails / notifications when attempting to silently move Topics between categories

Bug
,
1

Just got burned when moving 30 topics between categories by an email storm to my most valuable users.

I noticed after the last update that the text has changed for the feature which allows the suppression of notifications when a bulk selected group of Topics have their Category changed. It now shows this:

image
image786Ɨ237 5.75 KB

I left that unchecked, and it emailed them like crazy! The destination category is configured as Watching first post, but before this change those would have been suppressed.

What is going on here?

5 Likes
2

Sorry about that @nathank :frowning:

Iā€™ve tried to reproduce locally, and improved our spec coverage to ensure we handled this caseā€¦

ā€¦ but everything seems to be working as expected :thinking:

Any chances you could find some reproductions steps?

1 Like
3

@zogstrip I donā€™t see test coverage for this case.

In my reading, the describe "silent option" specs only cover the scenarios where the topic being moved is already watched.

The case being described here is for the scenario where the destination category is ā€œwatchedā€ or ā€œwatching first postā€.

@nathank, to my knowledge, weā€™ve never done that silently.

I think whatā€™s going on here is that the modal for the feature we did add makes it sound like that should also be silenced. I think weā€™ve shifted expectations with this modal and now weā€™re not meeting them.

Thatā€™s my hunch anyway.

I think this is a legitimate Feature or UX request, and I fully support doing it. But I donā€™t think itā€™s technically a bug or a regression.

2 Likes
6

(post deleted by author)

7

I had thought this was part of the purpose of including this as an option in bulk actions (to match the behaviour of Disable category edit notifications and Disable tag edit notifications on an ad hoc basis).

Without it, the feature would rather limited.

2 Likes
8

Good point. Iā€™m honestly not sure.

I do see thatā€™s exactly the use case described in the original feature request:

I also see that several folks thought the same thing when they saw the new checkbox: Bulk editing topic categories should not trigger thousands of email notifications - #12 by mbauman

The PR itself also makes it sound like that was the intention:

When the ā€œPerform this action silentlyā€ checkbox has been checked, the :notify_category_change sidekiq job should not be enqueued.

But Iā€™m not seeing explicit spec coverage for this case.

Admittedly, Iā€™m a bit out of practice readying ruby specs since I havenā€™t write code day to day for the past few years, but this does feel like a possible gap.

Smells more like a bug to me now. Not sure if regression or not, but does feel like weā€™re missing coverage for this case.

3 Likes
10

Ahah! I think Iā€™ve found the issue :bug:

The @silent option wasnā€™t propagating properly which caused users watching the destination category to receive notifications even when the ā€œPerform this action silentlyā€ checkbox was checked.

6 Likes
11

Brilliant! And thank you for both taking this seriously and the super quick fix! Iā€™ve just tested it and all seems good.

I just wanted to note that the text in the UI is now different / flipped - so that the ticking the checkbox does the exact opposite of what it used to do. But that side of things seems to be working fine, so hopefully all good.

It used to be:

It is now:

1 Like
12

:telephone: Please listen carefully, as our menu options have changedā€¦

2 Likes