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 breaking email restrictions checker in doctor #19903

Merged
merged 5 commits into from
Jun 7, 2022

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Jun 6, 2022

  • This patch introduces a new kind of doctor type, breaking. This file is made to register checks that helps with detecting when a breaking change might impact a Gitea instance.
  • For now the only check here(and the reason of creating this) is to check if all users in the database has a valid email address, which might not be the case after Restrict email address validation #17688. This simply uses the validation function to detect and report these cases.
  • Helps admins with detecting error 500 on login - invalid email address #19897.
  • I have no clue which priority this should be.

image

- This patch introduces a new kind of doctor type, breaking. This file
is made to register checks that helps with detecting when a breaking
change might impact a Gitea instance.
- For now the only check here(and the reason of creating this) is to
check if all users in the database has a valid email address, which
might not be the case after
go-gitea#17688. This _simply_ uses the
validation function to detect and report these cases.
- Helps admins with detecting go-gitea#19897.
- I have no clue which priority should be and IsDefault is true, because
when breaking change happen and we have a doctor check for it, we can
say "run `gitea doctor` to help you with this and maybe you find other
errors 😉".
@Gusted Gusted added this to the 1.17.0 milestone Jun 6, 2022
@Gusted Gusted added the type/enhancement An improvement of existing functionality label Jun 6, 2022
@Gusted Gusted changed the title Add breaking change check in doctor Add breaking email restrictions checker in doctor Jun 6, 2022
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@3d9c02a). Click here to learn what that means.
The diff coverage is 19.51%.

@@           Coverage Diff           @@
##             main   #19903   +/-   ##
=======================================
  Coverage        ?   47.33%           
=======================================
  Files           ?      961           
  Lines           ?   133716           
  Branches        ?        0           
=======================================
  Hits            ?    63291           
  Misses          ?    62715           
  Partials        ?     7710           
Impacted Files Coverage Δ
modules/doctor/breaking.go 19.51% <19.51%> (ø)

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 3d9c02a...cd96ac7. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 6, 2022
@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 Jun 7, 2022
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@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 Jun 7, 2022
@techknowlogick techknowlogick merged commit 59fd864 into go-gitea:main Jun 7, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 9, 2022
* giteaofficial/main:
  Prevent NPE whilst migrating if there is a team request review (go-gitea#19855)
  [skip ci] Updated translations via Crowdin
  Add support for rendering terminal output with colors (go-gitea#19497)
  Fix viewed images not loading in a PR (go-gitea#19919)
  Remove out-dated comments (go-gitea#19921)
  Automatically render wiki TOC (go-gitea#19873)
  Improve wording on delete access token modal (go-gitea#19909)
  [skip ci] Updated translations via Crowdin
  Add breaking email restrictions checker in doctor (go-gitea#19903)
  Ensure minimum mirror interval is reported on settings page (go-gitea#19895)
  Improve UX on modal for deleting an access token (go-gitea#19894)
  update discord invite (go-gitea#19907)
  Only log non ErrNotExist errors in git.GetNote  (go-gitea#19884)
  [skip ci] Updated translations via Crowdin
  Update frontend guideline (go-gitea#19901)
  Make AppDataPath absolute against the AppWorkPath if it is not (go-gitea#19815)
@Gusted Gusted deleted the add-email-check branch July 11, 2022 08:18
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
* Add breaking change check in doctor

- This patch introduces a new kind of doctor type, breaking. This file
is made to register checks that helps with detecting when a breaking
change might impact a Gitea instance.
- For now the only check here(and the reason of creating this) is to
check if all users in the database has a valid email address, which
might not be the case after
go-gitea#17688. This _simply_ uses the
validation function to detect and report these cases.
- Helps admins with detecting go-gitea#19897.
- I have no clue which priority should be and IsDefault is true, because
when breaking change happen and we have a doctor check for it, we can
say "run `gitea doctor` to help you with this and maybe you find other
errors 😉".

* Makes no sense tbh

* Fix copyright

* Update modules/doctor/breaking.go

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

Co-authored-by: techknowlogick <matti@mdranta.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 May 3, 2023
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. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants