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

Add wp_block_hash field #923

Merged

Conversation

suecarmol
Copy link
Contributor

Description

Added the wp_block_hash field and validated if the block has has changed everytime the user logs in

Rationale

We want a system for reviewing users with ignored block status in the case that their block status changes, so that we can evaluate if they should still be added to the allow list.

Phabricator Ticket

T295608

How Has This Been Tested?

Screenshots of your changes (if appropriate):

Types of changes

What types of changes does your code introduce? Add an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Minor change (fix a typo, add a translation tag, add section to README, etc.)

@suecarmol suecarmol force-pushed the suecarmol/T295608-block-status-change branch from 56830e5 to 5c450dd Compare January 20, 2022 17:46
@suecarmol suecarmol changed the title WIP - Add wp_block_hash field Add wp_block_hash field Jan 21, 2022
@suecarmol suecarmol force-pushed the suecarmol/T295608-block-status-change branch from 6f89413 to 2827c71 Compare January 21, 2022 02:05
@suecarmol suecarmol force-pushed the suecarmol/T295608-block-status-change branch from 2827c71 to 8cae0a6 Compare January 21, 2022 17:56
Copy link
Member

@jsnshrmn jsnshrmn left a comment

Choose a reason for hiding this comment

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

Thanks for all of your work on this! It is looking really good. There is only one thing that sticks out at me, and it's not actually a problem. It's the fact that the email send is setup directly in the user helper. We're using signals for sending emails in other places, but I don't think there is any advantage to registering a signal here; it has me wondering why I implemented so many of the other signals. Great work!

@jsnshrmn jsnshrmn merged commit cb91af0 into WikipediaLibrary:master Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants