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

change update visiblity ux #25856

Closed
wants to merge 12 commits into from
Closed

change update visiblity ux #25856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
157a25e
change update visiblity ux
mohammad258852 Jul 12, 2023
47e7ec7
Merge branch 'main' into visiblity
mohammad258852 Jul 13, 2023
e4e9bd4
Merge branch 'main' into visiblity
mohammad258852 Jul 22, 2023
3d01c46
move change visibility to danger zone
mohammad258852 Aug 3, 2023
2757fa3
fix a bug
mohammad258852 Aug 3, 2023
0df63a8
try not to combine label
mohammad258852 Aug 3, 2023
1801af3
remove safe function
mohammad258852 Aug 3, 2023
58064bb
Merge branch 'main' into visiblity
mohammad258852 Aug 3, 2023
c069a83
Merge branch 'main' into visiblity
mohammad258852 Oct 3, 2023
5b23f3a
fix style
mohammad258852 Oct 3, 2023
a4c5209
Merge branch 'main' into visiblity
mohammad258852 Oct 3, 2023
a11b4ae
Merge branch 'main' into visiblity
GiteaBot Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,9 +937,12 @@ template_helper = Make repository a template
template_description = Template repositories let users generate new repositories with the same directory structure, files, and optional settings.
visibility = Visibility
visibility_description = Only the owner or the organization members if they have rights, will be able to see it.
visibility_helper = Make repository private
visibility_helper_private = Make Repository Private
visibility_helper_public = Make Repository Public
visibility_helper_forced = Your site administrator forces new repositories to be private.
visibility_fork_helper = (Changing this will affect all forks.)
visibility_fork_helper = Changing this will affect all forks.
visibility_credential = With making repository public every one can see your code including imporant data in it.
visibility.update_error = For making repository private go to danger zone
clone_helper = Need help cloning? Visit <a target="_blank" rel="noopener noreferrer" href="%s">Help</a>.
fork_repo = Fork Repository
fork_from = Fork From
Expand Down
47 changes: 38 additions & 9 deletions routers/web/repo/setting/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,7 @@ func SettingsPost(ctx *context.Context) {
form.Private = repo.BaseRepo.IsPrivate || repo.BaseRepo.Owner.Visibility == structs.VisibleTypePrivate
}

visibilityChanged := repo.IsPrivate != form.Private
// when ForcePrivate enabled, you could change public repo to private, but only admin users can change private to public
if visibilityChanged && setting.Repository.ForcePrivate && !form.Private && !ctx.Doer.IsAdmin {
ctx.RenderWithErr(ctx.Tr("form.repository_force_private"), tplSettingsOptions, form)
return
}

repo.IsPrivate = form.Private
if err := repo_service.UpdateRepository(ctx, repo, visibilityChanged); err != nil {
if err := repo_service.UpdateRepository(ctx, repo, false); err != nil {
ctx.ServerError("UpdateRepository", err)
return
}
Expand Down Expand Up @@ -865,6 +857,43 @@ func SettingsPost(ctx *context.Context) {
ctx.Flash.Success(ctx.Tr("repo.settings.wiki_deletion_success"))
ctx.Redirect(ctx.Repo.RepoLink + "/settings")

case "change-visiblity":
if ctx.HasError() {
ctx.HTML(http.StatusOK, tplSettingsOptions)
return
}
if !ctx.Repo.IsOwner() {
ctx.Error(http.StatusNotFound)
return
}
Comment on lines +865 to +868
Copy link
Contributor

Choose a reason for hiding this comment

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

The old code doesn't have such check.

IIRC the IsOwner should have been correctly checked by a middleware for this handler.

if repo.Name != form.RepoName {
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil)
return
}

private := form.Private

if repo.IsFork {
private = repo.BaseRepo.IsPrivate || repo.BaseRepo.Owner.Visibility == structs.VisibleTypePrivate
}

visibilityChanged := repo.IsPrivate != private
if visibilityChanged && setting.Repository.ForcePrivate && !private && !ctx.Doer.IsAdmin {
Copy link
Contributor

Choose a reason for hiding this comment

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

A useful comment is lost

// when ForcePrivate enabled, you could change public repo to private, but only admin users can change private to public

ctx.RenderWithErr(ctx.Tr("form.repository_force_private"), tplSettingsOptions, form)
return
}

repo.IsPrivate = private
if err := repo_service.UpdateRepository(ctx, repo, visibilityChanged); err != nil {
ctx.ServerError("UpdateRepository", err)
return
}

log.Trace("Repository visibility settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name)

ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success"))
ctx.Redirect(ctx.Repo.RepoLink + "/settings")

case "archive":
if !ctx.Repo.IsOwner() {
ctx.Error(http.StatusForbidden)
Expand Down
91 changes: 83 additions & 8 deletions templates/repo/settings/options.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,7 @@
{{if not .Repository.IsFork}}
<div class="inline field">
<label>{{ctx.Locale.Tr "repo.visibility"}}</label>
<div class="ui checkbox" {{if and (not .Repository.IsPrivate) (gt .Repository.NumStars 0)}}data-tooltip-content="{{ctx.Locale.Tr "repo.stars_remove_warning"}}"{{end}}>
{{if .IsAdmin}}
<input name="private" type="checkbox" {{if .Repository.IsPrivate}}checked{{end}}>
{{else}}
<input name="private" type="checkbox" {{if .Repository.IsPrivate}}checked{{end}}{{if and $.ForcePrivate .Repository.IsPrivate}} readonly{{end}}>
{{end}}
<label>{{ctx.Locale.Tr "repo.visibility_helper" | Safe}} {{if .Repository.NumForks}}<span class="text red">{{ctx.Locale.Tr "repo.visibility_fork_helper"}}</span>{{end}}</label>
</div>
<span>{{if .Repository.IsPrivate}}{{ctx.Locale.Tr "settings.visibility.private"}}{{else}}{{ctx.Locale.Tr "settings.visibility.public"}}{{end}}</span>
</div>
{{end}}
<div class="field {{if .Err_Description}}error{{end}}">
Expand Down Expand Up @@ -763,6 +756,44 @@
</div>
</div>
{{end}}

{{if not .Repository.IsFork}}
<div class="divider"></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this divider (and below) is needed? IIRC the flex-item has proper border-top: 1px solid var(--color-secondary);.


<div class="flex-item">
<div class="flex-item-main">
<div class="flex-item-title">
{{if not .Repository.IsPrivate}}
{{ctx.Locale.Tr "repo.visibility_helper_private"}}
{{else}}
{{ctx.Locale.Tr "repo.visibility_helper_public"}}
{{end}}
</div>
<div class="flex-item-body">
{{if and (not .Repository.IsPrivate) (gt .Repository.NumStars 0)}}
{{ctx.Locale.Tr "repo.stars_remove_warning"}}
{{end}}
{{if and (not .Repository.IsPrivate) (.Repository.NumForks)}}
{{ctx.Locale.Tr "repo.visibility_fork_helper"}}
{{end}}
{{if .Repository.IsPrivate}}
{{ctx.Locale.Tr "repo.visibility_credential"}}
{{end}}
</div>
</div>
<div class="flex-item-trailing">
<button class="ui basic red show-modal button" data-modal="#repo-visibility-modal">
{{if not .Repository.IsPrivate}}
{{ctx.Locale.Tr "repo.visibility_helper_private"}}
{{else}}
{{ctx.Locale.Tr "repo.visibility_helper_public"}}
{{end}}
</button>
</div>
</div>
{{end}}
<div class="divider"></div>

<div class="flex-item">
<div class="flex-item-main">
<div class="flex-item-title">{{ctx.Locale.Tr "repo.settings.delete"}}</div>
Expand Down Expand Up @@ -966,6 +997,50 @@
</div>
{{end}}

{{if not .Repository.IsFork}}
<div class="ui small modal" id="repo-visibility-modal">
<div class="header">
{{ctx.Locale.Tr "repo.visibility"}}
</div>
<div class="content">
<div class="ui warning message">
{{if not .Repository.IsPrivate}}
{{ctx.Locale.Tr "repo.stars_remove_warning"}}<br>
{{ctx.Locale.Tr "repo.visibility_fork_helper"}}
{{else}}
{{ctx.Locale.Tr "repo.visibility_credential"}}
{{end}}
</div>
<form class="ui form" action="{{.Link}}" method="post">
{{.CsrfTokenHtml}}
<input type="hidden" name="action" value="change-visiblity">
<input type="hidden" name="private" value={{if .Repository.IsPrivate}}"false"{{else}}"true"{{end}}>
<div class="field">
<label>
{{ctx.Locale.Tr "repo.settings.transfer_form_title"}}
<span class="text red">{{.Repository.Name}}</span>
</label>
</div>
<div class="required field">
<label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" required>
Copy link
Contributor

Choose a reason for hiding this comment

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

There are many id="repo_name" elements in this page. The "id" should be unique.

</div>

<div class="text right actions">
<button class="ui cancel button">{{ctx.Locale.Tr "settings.cancel"}}</button>
<button class="ui red button">
{{if not .Repository.IsPrivate}}
{{ctx.Locale.Tr "repo.visibility_helper_private"}}
{{else}}
{{ctx.Locale.Tr "repo.visibility_helper_public"}}
{{end}}
</button>
</div>
</form>
</div>
</div>
{{end}}

{{if not .Repository.IsMirror}}
<div class="ui g-modal-confirm modal" id="archive-repo-modal">
<div class="header">
Expand Down