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.

Sign up for GitHub

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

Jump to bottom

Move repository visibility to danger zone in the settings area #31126

Merged
merged 13 commits into from
Aug 11, 2024
Merged

Move repository visibility to danger zone in the settings area #31126

merged 13 commits into from
Aug 11, 2024

Conversation

fabiobarkoski
Copy link
Contributor

@fabiobarkoski fabiobarkoski commented May 27, 2024 •
edited

Moved repository visibility to the danger zone in the settings area. To change the visibility, it is necessary to go to the danger zone, click on the private/public button, and accept the change in the modal.

Resolves: #23826

Screenshots

Before Private repo:

Private repo

Public repo:

Public repo

After Make private:

Screenshot from 2024-05-28 21-35-38

Make private modal

Screenshot from 2024-06-13 23-55-55

Screenshot from 2024-06-13 23-53-09

Make public:

Screenshot from 2024-05-28 21-34-27

Make public modal

Screenshot from 2024-06-13 23-53-37

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 27, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 27, 2024
@github-actions github-actions bot added modifies/translation modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels May 27, 2024
@yp05327 yp05327 added this to the 1.23.0 milestone May 28, 2024
@lunny
Copy link
Member

lunny commented May 28, 2024

The warning should have more details. Make a repository private may remove the relation between it and forks, watchers and stars.

kemzeb
kemzeb reviewed May 28, 2024
View reviewed changes
options/locale/locale_en-US.ini Outdated Show resolved Hide resolved
options/locale/locale_en-US.ini Outdated Show resolved Hide resolved
@lunny
Copy link
Member

lunny commented May 29, 2024 •
edited

I would like the backend code should also be changed. There should be two new functions at service layer named MakeRepoPublic and MakeRepoPrivate.

@fabiobarkoski
Copy link
Contributor Author

Sure, but isn't it better just one function named ChangeVisibility? Since it will make almost the same action.

@lunny
Copy link
Member

lunny commented May 30, 2024

Sure, but isn't it better just one function named ChangeVisibility? Since it will make almost the same action.

They are opposite actions, not the same. I don't think they will share much code.

Yakov5776
Yakov5776 reviewed May 30, 2024
View reviewed changes
options/locale/locale_en-US.ini
@@ -2446,6 +2446,13 @@ settings.thread_id = Thread ID
settings.matrix.homeserver_url = Homeserver URL
settings.matrix.room_id = Room ID
settings.matrix.message_type = Message Type
settings.visibility.private.button = Make Private
settings.visibility.private.text = Changing the visibility to private will not only make the repo visible to only allowed members but may remove the relation between it and forks, watchers, and stars.
Copy link
Contributor

@Yakov5776 Yakov5776 May 30, 2024 •
edited

Choose a reason for hiding this comment

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

IMO, It might be more understandable and user friendly to display this warning as bullet marks like:

Changing the visibility to private will:

  • Make the repository only visible to yourself and members who have access
  • Remove any affiliation between this repository and any forks, watchers, and stars.

Copy link
Contributor Author

@fabiobarkoski fabiobarkoski May 31, 2024 •
edited

Choose a reason for hiding this comment

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

Agree, but I think will need to change the archive text to bullet marks too, to keep the consistency.

@fabiobarkoski
Copy link
Contributor Author

They are opposite actions, not the same. I don't think they will share much code.

Yeah, you're right, I've expressed myself badly.

I would like the backend code should also be changed. There should be two new functions at service layer named MakeRepoPublic and MakeRepoPrivate.

Both using the UpdateRepository from modules or in the same way create a function for each?

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 14, 2024
@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 Aug 9, 2024
@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 Aug 9, 2024
fabiobarkoski and others added 6 commits August 9, 2024 17:02
@fabiobarkoski
Move repository visibility to danger zone in the settings area
e3f44ad 
Moved to danger zone because change the visibility is a
non-trivial decision, so that way the repo admin need
to go to the danger zone to change.
@fabiobarkoski
Change private/public to visibility
39a75a6
@fabiobarkoski @kemzeb
Apply suggestions from code review
f938285 
Co-authored-by: Kemal Zebari <60799661+kemzeb@users.noreply.github.com>
@fabiobarkoski
Increase private warning details
987f734
@fabiobarkoski
Create functions in the service layer to change the visibility
d0a77f9
@fabiobarkoski
Display warnings as bullet marks
b4c16e5
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 9, 2024
@lafriks lafriks merged commit e45a4c9 into go-gitea:main Aug 11, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 11, 2024
@lunny lunny mentioned this pull request Aug 11, 2024
Closed
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 11, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
494abf6 
* giteaofficial/main:
  Show lock owner instead of repo owner on LFS setting page (go-gitea#31788)
  Move repository visibility to danger zone in the settings area (go-gitea#31126)
  [skip ci] Updated translations via Crowdin
  Add types to various low-level functions (go-gitea#31781)
  Add warning message in merge instructions when `AutodetectManualMerge` was not enabled (go-gitea#31805)
  Show latest run when visit /run/latest (go-gitea#31808)
  Fix typo for `LOG_COMPRESSION` in ini (go-gitea#31809)
  Add label `docs-update-needed` for PRs that modify `app.example.ini` (go-gitea#31810)
  Fix `IsObjectExist` with gogit (go-gitea#31790)
  Support compression for Actions logs (go-gitea#31761)
  Add issue comment when moving issues from one column to another of the project (go-gitea#29311)
  [skip ci] Updated translations via Crowdin
  Fix RPM resource leak (go-gitea#31794)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silverwind silverwind silverwind left review comments

@yp05327 yp05327 yp05327 left review comments

@Yakov5776 Yakov5776 Yakov5776 left review comments

@kemzeb kemzeb kemzeb left review comments

@lunny lunny lunny approved these changes

@lafriks lafriks lafriks approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

"Make Repository Private" option should be moved into the "Danger Zone" settings area
8 participants
@fabiobarkoski @lunny @silverwind @lafriks @yp05327 @Yakov5776 @kemzeb @GiteaBot