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

Invites: Filter invite object and decode entities #1258

Merged
merged 1 commit into from
Dec 4, 2015

Conversation

ebinnion
Copy link
Contributor

@ebinnion ebinnion commented Dec 4, 2015

This PR filters an invite and ensures that all value are decoded. Addresses #1086

To test:

  • Checkout update/invites-html-entities
  • Invite a test user by going to $site/wp-admin/users.php?page=wpcom-invite-users
  • In the invitation email that you get, make note of the $invite_key
  • Change the title of your site to include an entity of some sort ( ex. &)
  • Go to /accept-invite/$site/$invite_key where $site has an entity in its name
  • Is your site's title decoded properly?

Note: Decoding the user object is now taking place in #1257, so the entities showing in the below screenshot are OK.

screen shot 4

@ebinnion ebinnion added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. People Management labels Dec 4, 2015
@ebinnion ebinnion self-assigned this Dec 4, 2015
@ebinnion ebinnion added this to the People Management: m6 milestone Dec 4, 2015

const initialState = fromJS( {
list: {},
errors: {}
} );

function filterObject( object ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could we change the name of this function for something more descriptive? something like decodeProperties or decodeObjectProperties or similar? with filterObject you don't really get what it does unless you look at the code of the function

@johnHackworth
Copy link
Contributor

It works fine! I left a minor comment about variable naming, but appart from that it's ready to merge... if you're agree with me, change the name & merge, and if not, merge right away! :D

@johnHackworth johnHackworth added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants