Skip to content

feat: added pagination, search, and description to org teams page#36602

Closed
kmranimesh wants to merge 9 commits intogo-gitea:mainfrom
kmranimesh:feat/34482-team-list-pagination-search
Closed

feat: added pagination, search, and description to org teams page#36602
kmranimesh wants to merge 9 commits intogo-gitea:mainfrom
kmranimesh:feat/34482-team-list-pagination-search

Conversation

@kmranimesh
Copy link
Copy Markdown
Contributor

Added pagination and keyword search to the organization teams list page,
and displayed team descriptions in the team cards.

Add pagination and keyword search to the organization teams list page.
Display team descriptions in team cards.

- Refactor Teams() handler to use SearchTeam() with ListOptions
  for server-side pagination and keyword filtering
- Add search input with the existing 'search.team_kind' locale string
- Show team description in an attached segment below the team name
- Add standard pagination widget at the bottom of the list

Fixes go-gitea#34482
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 12, 2026
@silverwind silverwind requested a review from Copilot February 12, 2026 18:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds server-side pagination and keyword search to the organization teams list, and displays team descriptions on the teams page to improve usability for orgs with many teams.

Changes:

  • Refactors Teams() handler to use org_model.SearchTeam() with ListOptions for pagination and keyword filtering.
  • Adds a search input (q) to filter teams by name/description.
  • Displays team descriptions in team cards and adds the standard pagination widget.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
templates/org/team/teams.tmpl Adds search UI, renders team descriptions, and includes the pagination widget.
routers/web/org/teams.go Implements paginated + keyword-filtered team retrieval via SearchTeam() and wires up pagination state to the template.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread routers/web/org/teams.go Outdated
Comment thread templates/org/team/teams.tmpl Outdated
@lunny
Copy link
Copy Markdown
Member

lunny commented Feb 12, 2026

Could you drop a screenshot of that?

@silverwind
Copy link
Copy Markdown
Member

Review of this PR (this comment was written by Claude Code / claude-opus-4-6):

Permission regression for non-owner admin members. The org context middleware in services/context/org.go:174-205 has a shouldSeeAllTeams check: non-owners who belong to a team with IncludesAllRepositories && HasAdminAccess() are allowed to see all org teams, not just their own. The PR's handler logic only checks ctx.Org.IsOwner to decide whether to set opts.UserID, so these non-owner admins would be downgraded to seeing only their own teams. This should replicate the middleware's shouldSeeAllTeams logic, or reuse ctx.Org.Teams to derive the visibility.

Redundant full team loading. The middleware still runs before the handler and loads all teams into ctx.Org.Teams (unpaginated) on every request. The handler then runs its own paginated SearchTeam query. This means every page load does two separate team queries — the middleware's full load plus the handler's paginated one. This isn't incorrect but is wasteful. Ideally the middleware's unconditional full load could be avoided for this route, but that may be a larger refactor since ctx.Org.Teams is also used for team name resolution and other purposes.

Pagination wiring looks correct. AddParamFromRequest preserves the q keyword across page links, setting.UI.MembersPagingNum (default 20) is a reasonable page size, and {{template "base/paginate" .}} uses .Page which is correctly set.

Template changes look fine. The description segment (ui attached segment) with {{.Description}} is auto-escaped by Go templates. Moving the divider outside the {{if .IsOrganizationOwner}} block is correct since non-owners also see the search bar now.

Minor: the search bar is always shown, even for non-owners. This is fine UX-wise — it lets any org member search their visible teams.

@kmranimesh
Copy link
Copy Markdown
Contributor Author

Could you drop a screenshot of that?

Yes, sure, I'm attaching the screenshots

Screenshot 2026-02-13 at 9 29 14 PM Screenshot 2026-02-13 at 9 29 35 PM Screenshot 2026-02-13 at 9 31 15 PM

Comment thread templates/org/team/teams.tmpl Outdated
Signed-off-by: silverwind <me@silverwind.io>
Comment thread templates/org/team/teams.tmpl Outdated
@silverwind
Copy link
Copy Markdown
Member

Comment authored by @silverwind via Claude Code.

Follow-up review after the latest updates:

The permission regression I flagged earlier is now properly addressed — CanUserSeeAllTeams in models/organization/org.go correctly replicates the original shouldSeeAllTeams logic (admin, owner, or member of a team with IncludesAllRepositories && HasAdminAccess()), and both the middleware in services/context/org.go and the handler in routers/web/org/teams.go use it. The refactor is clean and eliminates the duplicated inline logic.

Remaining notes:

  1. Double team loading: The middleware still unconditionally loads all teams into ctx.Org.Teams (used for NumTeams in the org header tab and team name resolution from URL paths). The handler then runs a second paginated SearchTeam query. This means two team queries per page load. Not incorrect, but worth being aware of for orgs with many teams. Fixing this would require a larger middleware refactor since other routes depend on ctx.Org.Teams, so not a blocker for this PR.

  2. Pagination and search wiring looks correct. pager.AddParamFromRequest preserves the q keyword across page links. setting.UI.MembersPagingNum is a reasonable page size choice given there is no dedicated teams paging setting.

  3. Template: The description segment (ui attached segment with {{.Description}}) is properly auto-escaped. The search form using type="search" with maxlength="255" and spellcheck="false" follows the pattern used elsewhere. Moving the divider outside the owner-only block is correct.

  4. Minor: CanUserSeeAllTeams loads all user teams just to check for admin+all-repos access. Same pattern as before, just refactored — not a regression but could be a single targeted query in the future.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Feb 18, 2026

Please address point 1 and maybe point 4.

Copy link
Copy Markdown
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Review (written by Claude Code / claude-opus-4-6):

  1. Code duplication: The identical 14-line team-loading block is copy-pasted in both routers/web/org/home.go and routers/web/user/home.go. Should be extracted into a shared helper like Organization.LoadVisibleTeams(ctx, user).

  2. CanUserSeeAllTeams called multiple times per request: On the teams page it's called in the middleware (for NumTeams), potentially again in the middleware (for :team path resolution), and again in the Teams() handler. Each call hits the DB. The result should be computed once and stored on ctx.Org.

  3. No tests: CanUserSeeAllTeams (especially the non-owner admin-team path), paginated Teams() handler, and CountTeam have no test coverage.

@silverwind
Copy link
Copy Markdown
Member

1 and 2 should be addressed, then it's lgtm. Tests would be nice but not a blocker from me.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Although it might work, the implementation seems to be a hacky patch to existing ctx.Org.Teams.

ctx.Org.Teams was designed to store all the teams for current doer. The design itself is not good enough, it just works.

But this PR introduces more changes and make the things more complicated (e.g.: many ctx.Org.Teams == nil checks and logic).

TBH I have difficulty to understand why it should be implemented like this, and no idea about how the code can be maintained in the future.

@wxiaoguang wxiaoguang marked this pull request as draft February 22, 2026 11:52
@wxiaoguang wxiaoguang closed this Apr 17, 2026
silverwind added a commit that referenced this pull request Apr 17, 2026
- Add pagination and keyword search to the teams list page
- 5 teams shown at most in the overview page

Fixes: #34482
Fixes: #36602
Fixes: #37084
Signed-off-by: silverwind <me@silverwind.io>
Co-authored-by: Animesh Kumar <83393501+kmranimesh@users.noreply.github.com>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Paginate, filter and display descriptions of teams in the organization page

6 participants