Skip to content
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

Filter out moderators for moderator email notifications #9269

Merged
merged 2 commits into from
Mar 16, 2021

Conversation

17sushmita
Copy link
Member

@17sushmita 17sushmita commented Mar 4, 2021

Fixes #9114

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • ask @publiclab/reviewers for help, in a comment below

@gitpod-io
Copy link

gitpod-io bot commented Mar 4, 2021

@17sushmita
Copy link
Member Author

@jywarren, I could not find any tag named notifications:noemail. So I've filtered out only tag named no-moderation-emails. Can you review this PR?

@codecov
Copy link

codecov bot commented Mar 4, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@30a6a38). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9269   +/-   ##
=======================================
  Coverage        ?   82.28%           
=======================================
  Files           ?       98           
  Lines           ?     5864           
  Branches        ?        0           
=======================================
  Hits            ?     4825           
  Misses          ?     1039           
  Partials        ?        0           

Copy link
Member

@jywarren jywarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, just the one tweak in the conditional and we can merge! I'm going to resolve/ignore the CodeClimate requests as the code looks pretty tidy. Thanks!!

app/mailers/admin_mailer.rb Outdated Show resolved Hide resolved
app/mailers/admin_mailer.rb Outdated Show resolved Hide resolved
app/mailers/admin_mailer.rb Outdated Show resolved Hide resolved
app/mailers/admin_mailer.rb Outdated Show resolved Hide resolved
app/mailers/admin_mailer.rb Outdated Show resolved Hide resolved
app/mailers/admin_mailer.rb Outdated Show resolved Hide resolved
@codeclimate
Copy link

codeclimate bot commented Mar 12, 2021

Code Climate has analyzed commit b6c8d25 and detected 10 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 4
Style 6

View more on Code Climate.

Copy link
Member

@jywarren jywarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thank you so much!!!!

@jywarren jywarren merged commit c9349ca into publiclab:main Mar 16, 2021
@jywarren
Copy link
Member

Excellent work!!!! Thanks everyone for the reviews as well! 🎉

reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this pull request Oct 16, 2021
* Filter out moderators for moderator email notifications

* Added notifications:noemail tag to filter moderator emails
billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this pull request Dec 28, 2021
* Filter out moderators for moderator email notifications

* Added notifications:noemail tag to filter moderator emails
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants