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

skip email validation on empty string #13627

Merged
merged 2 commits into from
Nov 20, 2020

Conversation

6543
Copy link
Member

@6543 6543 commented Nov 18, 2020

  • move validation into its own function
  • use a session for UpdateUserSetting

close #13587
close #13653

@6543 6543 added type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Nov 18, 2020
@6543 6543 modified the milestones: 1.13.0, 1.14.0 Nov 18, 2020
@codecov-io
Copy link

codecov-io commented Nov 18, 2020

Codecov Report

Merging #13627 (eb3e1e4) into master (1bb5c09) will increase coverage by 0.00%.
The diff coverage is 45.83%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #13627   +/-   ##
=======================================
  Coverage   42.20%   42.20%           
=======================================
  Files         697      697           
  Lines       76589    76597    +8     
=======================================
+ Hits        32325    32331    +6     
+ Misses      38948    38947    -1     
- Partials     5316     5319    +3     
Impacted Files Coverage Δ
models/user_mail.go 50.38% <40.00%> (-0.01%) ⬇️
models/user.go 54.34% <50.00%> (-0.23%) ⬇️
modules/queue/unique_queue_disk_channel.go 53.84% <0.00%> (-1.54%) ⬇️
models/gpg_key.go 53.90% <0.00%> (+0.57%) ⬆️
modules/queue/workerpool.go 60.00% <0.00%> (+1.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bb5c09...eb3e1e4. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 18, 2020
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 19, 2020
@6543
Copy link
Member Author

6543 commented Nov 19, 2020

regression of #13475 - so I'll add skip-changelog label

@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Nov 19, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 19, 2020
models/user_mail.go Outdated Show resolved Hide resolved
Copy link
Member

@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

As per my comment

- move validation into its own function
- use a session for UpdateUserSetting
@6543 6543 force-pushed the fix_refactor-validate-email branch from fcaa90d to 5aab44f Compare November 19, 2020 23:21
@6543
Copy link
Member Author

6543 commented Nov 19, 2020

@lafriks done

@lafriks lafriks merged commit f915161 into go-gitea:master Nov 20, 2020
@lafriks lafriks deleted the fix_refactor-validate-email branch November 20, 2020 21:46
6543 added a commit to 6543-forks/gitea that referenced this pull request Nov 21, 2020
- move validation into its own function
- use a session for UpdateUserSetting
techknowlogick added a commit that referenced this pull request Nov 22, 2020
* Add email validity check (#13475)

* Improve error feedback for duplicate deploy keys

Instead of a generic HTTP 500 error page, a flash message is rendered
with the deploy key page template so inform the user that a key with the
intended title already exists.

* API returns 422 error when key with name exists

* Add email validity checking

Add email validity checking for the following routes:
[Web interface]
1. User registration
2. User creation by admin
3. Adding an email through user settings
[API]
1. POST /admin/users
2. PATCH /admin/users/:username
3. POST /user/emails

* Add further tests

* Add signup email tests

* Add email validity check for linking existing account

* Address PR comments

* Remove unneeded DB session

* Move email check to updateUser

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>

* skip email validation on empty string (#13627)

- move validation into its own function
- use a session for UpdateUserSetting

* rm TODO for backport

Co-authored-by: Chris Shyi <chrisshyi13@gmail.com>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
5 participants