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

Fix default for allowing new organization creation for new users #7017

Merged
merged 3 commits into from May 24, 2019

Conversation

@jpicht
Copy link
Contributor

commented May 22, 2019

When creating users DefaultAllowCreateOrganization was ignored.

This is simple two-line fix for issue #6542.

FIX issue 6542
When creating users DefaultAllowCreateOrganization was ignored.

Signed-off-by: Julian Picht <julian.picht@gmail.com>
@zeripath

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

~~Looking at the code I think these two settings need to be coalesced. It doesn't appear that DEFAULT_ALLOW_CREATE_ORGANIZATION is wired into anything at all. ~~

@zeripath

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

We should probably just kill DEFAULT_ALLOW_CREATE_ORGANIZATION let's just rewire it back in and fix the test

@jpicht

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

I can certainly do that as well. Should there be a warning if somebody has it set in the config file? Is there a place to add a message to, so it will end up in the release notes?

@zeripath

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

Hmm... let's just fix the test. I think that's the correct thing to do.

Make line 263:

gitea/models/user_test.go

Lines 262 to 264 in 6eb53ac

}
for _, v := range tt {

setting.Service.DefaultAllowCreateOrganization = true
fix TestCreateUser_Issue5882
Signed-off-by: Julian Picht <julian.picht@gmail.com>
@jpicht

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

I ran the drone test locally before creating this pull request. It seems the test did not fail for me. What did I do wrong? The output can be found here: https://gist.github.com/jpicht/6337485ab466776fded3ee22089d45dc

@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels May 23, 2019

@zeripath

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

You didn't run the unit tests - that is make test - your logs indicate you only ran the integration tests

@codecov-io

This comment has been minimized.

Copy link

commented May 23, 2019

Codecov Report

Merging #7017 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7017      +/-   ##
==========================================
- Coverage    41.5%   41.48%   -0.02%     
==========================================
  Files         440      440              
  Lines       59453    59452       -1     
==========================================
- Hits        24675    24663      -12     
- Misses      31558    31570      +12     
+ Partials     3220     3219       -1
Impacted Files Coverage Δ
models/user.go 50.88% <100%> (-0.05%) ⬇️
models/unit.go 62.16% <0%> (-5.41%) ⬇️
modules/log/event.go 64.46% <0%> (-1.53%) ⬇️
models/repo_list.go 72.08% <0%> (-1.02%) ⬇️
routers/repo/view.go 42.02% <0%> (-1.02%) ⬇️

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 6eb53ac...7301c69. Read the comment docs.

@jpicht

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

You didn't run the unit tests - that is make test - your logs indicate you only ran the integration tests

Thanks! That step should probably be added to the contributors guide.

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels May 24, 2019

@lafriks lafriks added the kind/bug label May 24, 2019

@lafriks

This comment has been minimized.

Copy link
Member

commented May 24, 2019

Make lgtm work

@lafriks lafriks merged commit 8cd4c22 into go-gitea:master May 24, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details

@lafriks lafriks changed the title FIX issue #6542 Fix default for allowing new organization creation for new users May 24, 2019

@lafriks

This comment has been minimized.

Copy link
Member

commented May 24, 2019

Please send backport to release/v1.8 branch

lafriks added a commit that referenced this pull request May 24, 2019

Fix default for allowing new organization creation for new users (#7017
…) (#7034)

* FIX issue 6542

When creating users DefaultAllowCreateOrganization was ignored.

Signed-off-by: Julian Picht <julian.picht@gmail.com>

* fix TestCreateUser_Issue5882

Signed-off-by: Julian Picht <julian.picht@gmail.com>

jeffliu27 added a commit to jeffliu27/gitea that referenced this pull request Jul 18, 2019

Fix default for allowing new organization creation for new users (go-…
…gitea#7017)

Fixed go-gitea#6542

When creating users DefaultAllowCreateOrganization was ignored.

Signed-off-by: Julian Picht <julian.picht@gmail.com>

* fix TestCreateUser_Issue5882

Signed-off-by: Julian Picht <julian.picht@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.