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 org repo creation being limited by user limits #34030

Merged
merged 6 commits into from
Mar 27, 2025

Conversation

TheFox0x7
Copy link
Contributor

fixes an issue where user is unable to create new repository in organization via UI if repository limits are in place and user has exhausted them for their own namespace.

closes: #15504

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 26, 2025
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 26, 2025
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Mar 26, 2025
@TheFox0x7 TheFox0x7 changed the title fix org repo creating being limited by user limits fix org repo creation being limited by user limits Mar 26, 2025
@TheFox0x7
Copy link
Contributor Author

Docs for behavior difference:

Previous:

  1. User and Org has not exhausted their limits
    User can create repository in user namespace and in org namespace
  2. Org has exhausted limits
    User can create repo, org does not show up in the list
  3. User has exhausted limits but org did not
    Button is disabled and warning displayed on page load. User namespace and organization are displayed in the list
  4. Both exhausted their limits
    Button is disabled, warning displayed. User namespace is visible in the list but org is not.

Proposed:

  1. No changes
  2. No changes
  3. Button is enabled, warning shows up after user tries to create repository in their own namespace. Creating in org is not blocked
  4. Button is enabled, warning shows up for user namespace, org is not listed.

Writing this there's an alternative solution which is to remove user namespace from the list too (like orgs aren't added), but I think is this simpler and less confusing.

@wxiaoguang
Copy link
Contributor

Can we add some comments? IIRC this is a longstanding problem, many discussions, still unresolved .... (ps: fix ui bug when user can't create repo but org can #15924 )


ps: it looks good to me to leave the check to backend, but do not block user by the frontend.

@TheFox0x7
Copy link
Contributor Author

I'd like to give users an early warning that the action will fail but I think it's a more complicated solution which would end up needing JS or more understanding of frontend which I lack ATM. Maybe the page should be redone from scratch - no clue there.

Where should I add the comments (and anything in particular you'd want me to put in them)?

posting techknowlogick's comment so it's not lost in discord:

for your org limit PR I had a few Qs. I just wanted to make sure that even tho buttons may be enabled/disabled that if a user still attempted to create one and the namespace faced exhaustion, that it'd still be blocked. And I'm wondering about a possible PR that would be related, is blocking repo transfers if the accepting namespace is exhausted.

I've tested it manually and user gets the same error as before, just after submitting the form now.
Transfers aren't affected by limits at all from what I've tested

@wxiaoguang
Copy link
Contributor

I think it's a more complicated solution which would end up needing JS

I will take a try

@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 27, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 27, 2025
@wxiaoguang wxiaoguang force-pushed the fix-org-repo-creation branch from 39c9e97 to 1d8f02b Compare March 27, 2025 03:06
@wxiaoguang

This comment was marked as outdated.

@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 Mar 27, 2025
@wxiaoguang wxiaoguang added backport/v1.23 This PR should be backported to Gitea 1.23 type/bug labels Mar 27, 2025
@wxiaoguang
Copy link
Contributor

Not sure whether it could apply to 1.23 clearly. If there is conflict, I could backport it manually.

@wxiaoguang wxiaoguang force-pushed the fix-org-repo-creation branch from 1d8f02b to fbc1819 Compare March 27, 2025 03:34
@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 27, 2025
@wxiaoguang wxiaoguang force-pushed the fix-org-repo-creation branch from fbc1819 to bb7a83e Compare March 27, 2025 03:35
@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 Mar 27, 2025
@wxiaoguang
Copy link
Contributor

Wait for more feedbacks before merge.

@TheFox0x7
Copy link
Contributor Author

Case where user has no space left and there's no orgs with space either looks off to me:
image

The field is clickable but nothing happens on click. I don't think it's intended behavior

@wxiaoguang wxiaoguang marked this pull request as draft March 27, 2025 08:56
@wxiaoguang wxiaoguang self-assigned this Mar 27, 2025
@wxiaoguang
Copy link
Contributor

Finally, JS wins, and it is less tricky than the tmpl patches.

image

@wxiaoguang wxiaoguang marked this pull request as ready for review March 27, 2025 10:05
@wxiaoguang wxiaoguang force-pushed the fix-org-repo-creation branch from 0fe18a1 to bd78809 Compare March 27, 2025 10:10
@wxiaoguang wxiaoguang removed their assignment Mar 27, 2025
@wxiaoguang wxiaoguang force-pushed the fix-org-repo-creation branch from bd78809 to bfbc452 Compare March 27, 2025 10:28
@wxiaoguang wxiaoguang added this to the 1.24.0 milestone Mar 27, 2025
@wxiaoguang
Copy link
Contributor

Case where user has no space left and there's no orgs with space either looks off to me:
The field is clickable but nothing happens on click. I don't think it's intended behavior

Does the new change look good to you?

@TheFox0x7
Copy link
Contributor Author

Almost as it's now possible to submit the form and get two errors.
image

I guess since JS handles this now button could be disabled dynamically?

@wxiaoguang
Copy link
Contributor

I would like to keep them as-is, because these messages belong to different logic.

Or, maybe we could just disable the "submit" button, to avoid the duplicate error message?

@TheFox0x7
Copy link
Contributor Author

That's what I was suggesting - to disable submit button when user namespace is chosen (if appropriate)

@wxiaoguang
Copy link
Contributor

Done in the new commit:

image

@wxiaoguang wxiaoguang removed the backport/v1.23 This PR should be backported to Gitea 1.23 label Mar 27, 2025
@TheFox0x7
Copy link
Contributor Author

LGTM

@wxiaoguang wxiaoguang enabled auto-merge (squash) March 27, 2025 13:00
@wxiaoguang wxiaoguang merged commit 053592d into go-gitea:main Mar 27, 2025
26 checks passed
@lunny lunny added backport/done All backports for this PR have been created backport/v1.23 This PR should be backported to Gitea 1.23 labels Mar 28, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 31, 2025
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Add toggleClass function in dom.ts (go-gitea#34063)
  Add a config option to block "expensive" pages for anonymous users (go-gitea#34024)
  add additional ReplaceAll in pathsep to cater for different pathsep (go-gitea#34061)
  [skip ci] Updated translations via Crowdin
  enable staticcheck QFxxxx rules (go-gitea#34064)
  update to golangci-lint v2 (go-gitea#34054)
  Add descriptions for private repo public access settings and improve the UI (go-gitea#34057)
  Add anonymous access support for private/unlisted repositories (go-gitea#34051)
  Hide activity contributors, recent commits and code frequrency left tabs if there is no code permission (go-gitea#34053)
  Update action status badge layout (go-gitea#34018)
  Add anonymous access support for private repositories (backend) (go-gitea#33257)
  Simplify emoji rendering (go-gitea#34048)
  Adjust the layout of the toolbar on the Issues/Projects page (go-gitea#33667)
  Fix bug on downloading job logs (go-gitea#34041)
  Fix git client accessing renamed repo  (go-gitea#34034)
  Decouple Batch from git.Repository to simplify usage without requiring the creation of a Repository struct. (go-gitea#34001)
  fix org repo creation being limited by user limits (go-gitea#34030)
  Fix the issue with error message logging for the `check-attr` command on Windows OS. (go-gitea#34035)
  Try to fix check-attr bug (go-gitea#34029)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/done All backports for this PR have been created backport/v1.23 This PR should be backported to Gitea 1.23 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/bug
Projects
None yet
4 participants