-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Display all organization from user settings #1739
Conversation
LGTM |
LGTM, but I have not tried building this yet. |
Since it's a new feature, please merge this after v1.2 released |
{{template "base/alert" .}} | ||
<h4 class="ui top attached header"> | ||
{{.i18n.Tr "settings.orgs"}} | ||
<div class="ui right"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there should be if
to check if user has rights to create organizations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I will make a new commit for that.
options/locale/locale_fr-FR.ini
Outdated
@@ -274,6 +274,7 @@ applications=Applications | |||
orgs=Organisations | |||
delete=Supprimer le compte | |||
twofa=Authentification à deux facteurs | |||
organization = Organisation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only translate English :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed french translation
routers/user/setting.go
Outdated
func SettingsOrganization(ctx *context.Context) { | ||
ctx.Data["Title"] = ctx.Tr("settings") | ||
ctx.Data["PageIsSettingsOrganization"] = true | ||
orgs, err := models.GetOrgsByUserID(ctx.User.ID, ctx.IsSigned && ctx.User.IsAdmin) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why ctx.User.IsAdmin
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake, it should not be here. I pushed a new commit.
Other than my question, looks good. Why the |
For my point of view, it's faster to create that way. |
@DblK Builds fail, other than that we should be ready to merge :) |
Thanks for the review. Build failed because of time limit exceeded. Could you restart it? |
@DblK you can also commit --amend or rebase to restart a build. |
@lunny Sorry for merged this PR in |
This PR implement the display of all organizations that the current member is in.
Here are the UI modifications:
Feel free to comment. This is a port of a PR I made for gogs.