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 email validation in case of profile update #7304

Closed
wants to merge 1 commit into from

Conversation

golya
Copy link

@golya golya commented Sep 1, 2016

issue #7256

  • add email check in edit if necessary
  • edit method is refactored
  • getByEmail can accept "id" option.
  • add emailIsAlreadyInUse property to translations

Signed-off-by: Ádám Gólya adam.stork@gmail.com

@kirrg001
Copy link
Contributor

kirrg001 commented Sep 1, 2016

Hey @golya It would be great if you could add a test, which proofs that if we update a user and the email already exists, we expect a validation error to come back. You could add the test into the edit users section, see https://github.com/TryGhost/Ghost/blob/master/core/test/integration/api/api_users_spec.js#L449

Thanks for PR and for helping :)

issue TryGhost#7256
- add email check in edit if necessary
- edit method is refactored
- getByEmail can accept "id" option.
- add emailIsAlreadyInUse property to translations

Signed-off-by: Ádám Gólya <adam.stork@gmail.com>
}
},

update: function update(data, options) {

This comment was marked as abuse.

@@ -810,6 +828,9 @@ User = ghostBookshelf.Model.extend({

return Users.forge(options).fetch(options).then(function then(users) {
var userWithEmail = users.find(function findUser(user) {
if (options.id && parseInt(options.id) === user.get('id')) {

This comment was marked as abuse.

@kirrg001
Copy link
Contributor

kirrg001 commented Oct 5, 2016

@golya Hey :) Will you finish this PR for the next LTS release?Happens normally once at the of a month. Just asking to get an overview. Thanks for your help!

@kirrg001
Copy link
Contributor

kirrg001 commented Jan 4, 2017

@golya Hey. Would you like to finish your PR?

@golya
Copy link
Author

golya commented Jan 27, 2017

@kirrg001 Hi, sorry but I am really busy nowadays. I hope I will have some time in the future, but I can not promise anything. :(

@kirrg001
Copy link
Contributor

@golya

No problem 👍 I am going to close the PR for now to keep github clean. Your branch does not get deleted. If somebody else want's to fix this issue, we still can look at this as reference.
Feel free to re-create a PR if you have some time in the future. Thanks for your effort!

@kirrg001 kirrg001 closed this Jan 30, 2017
@janvt
Copy link
Contributor

janvt commented Jan 31, 2017

Will make suggested improvements and open another PR.

janvt pushed a commit to janvt/Ghost that referenced this pull request Jan 31, 2017
closes TryGhost#7256
- original code changes made by @golya in TryGhost#7304
- refactored edit method in user model, pulled update method from public API to prevent confusion
- added test coverage for existing email update in user model spec
- removed API test, as the current API does not allow adding new users, so not possible to test updating existing one
kirrg001 pushed a commit that referenced this pull request Feb 8, 2017
closes #7256

- original code changes made by @golya in #7304
- refactored edit method in user model to validate an existing email address
- added test coverage for existing email update in user model spec
kirrg001 pushed a commit that referenced this pull request Feb 8, 2017
closes #7256

- original code changes made by @golya in #7304
- refactored edit method in user model to validate an existing email address
- added test coverage for existing email update in user model spec
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

3 participants