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

Fix organization menu and packages details & refactor org menu #33928

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Fix organization menu and packages details & refactor org menu #33928

wants to merge 21 commits into from

Conversation

ExplodingDragon
Copy link
Contributor

@ExplodingDragon ExplodingDragon commented Mar 18, 2025 •
edited
Loading

Fix missing items in organization menu.

Menu

Show

Before:

After:

Packages

keep it consistent with the other pages.

Show

Before:

After:

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 18, 2025
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 18, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 18, 2025
@ExplodingDragon ExplodingDragon changed the title Fix org menu Title Fix org menu items Mar 18, 2025
@ExplodingDragon ExplodingDragon changed the title Fix org menu items Fix missing items in organization menu Mar 18, 2025
@ExplodingDragon ExplodingDragon marked this pull request as ready for review March 18, 2025 09:28
@lunny lunny added the type/bug label Mar 19, 2025
@lunny
Copy link
Member

lunny commented Mar 19, 2025

image
Org Code UI still missed the overview tab.

@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 20, 2025
wxiaoguang
wxiaoguang reviewed Mar 20, 2025
View reviewed changes
routers/web/user/code.go Outdated
ctx.ServerError("PrepareOrgHeader", err)
return
}
}
Copy link
Contributor

@wxiaoguang wxiaoguang Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Now the logic seems strange:

image

Ideally, maybe there should be a shared_user.PrepareOwnerHeader, and in it prepare for user or for org by user type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wxiaoguang I will attempt to refactor this section and extract more common code.

@ExplodingDragon ExplodingDragon marked this pull request as draft March 20, 2025 01:35
@wxiaoguang wxiaoguang mentioned this pull request Apr 6, 2025
Closed
@wxiaoguang
Copy link
Contributor

Is this PR active? Maybe it's good to have it in 1.24, ps: a recent similar PR:

@ExplodingDragon ExplodingDragon marked this pull request as ready for review April 8, 2025 02:21
@wxiaoguang
Copy link
Contributor

Thank you very much. I will try to add some asserts to make sure there won't be new regression in the future. Let me try.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 8, 2025 •
edited
Loading

Found more problems in old code. A lot of duplicate calls. Maybe we need to do more refactoring.

image

image

image

The mess started from "Count only visible repos on profile (#25928)", after that PR, the code is keeping being patched and copied-pasted.

@ExplodingDragon
Copy link
Contributor Author

All right. let me refactor again.

@ExplodingDragon ExplodingDragon marked this pull request as draft April 8, 2025 05:30
@wxiaoguang
Copy link
Contributor

All right. let me refactor again.

I could also help later 😄

@ExplodingDragon ExplodingDragon changed the title Fix missing items in organization menu Fix missing items in organization menu & refactor org menu Apr 8, 2025
@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 Apr 8, 2025
@ExplodingDragon
Copy link
Contributor Author

Note: Throwing in a bunch of unrelated variables could slow things down, but honestly, it's mostly just the UI, so no worries.

@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Apr 8, 2025
@ExplodingDragon ExplodingDragon changed the title Fix missing items in organization menu & refactor org menu Fix organization menu and packages details & refactor org menu Apr 8, 2025
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 8, 2025
@ExplodingDragon ExplodingDragon marked this pull request as ready for review April 8, 2025 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

Assignees
No one assigned
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ExplodingDragon @lunny @wxiaoguang @GiteaBot