Skip to content

Chore: Remove duplicated api calls in mirrors#695

Open
gesquivelgaghi wants to merge 3 commits into
mainfrom
chore/deduplicate-debarchive-apis
Open

Chore: Remove duplicated api calls in mirrors#695
gesquivelgaghi wants to merge 3 commits into
mainfrom
chore/deduplicate-debarchive-apis

Conversation

@gesquivelgaghi

@gesquivelgaghi gesquivelgaghi commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Summarize the changes made in this pull request. Include any relevant context or background information that would help reviewers understand the purpose and scope of the changes.

Release Impact

Pick the target branch (see RELEASES.md for the full model):

  • dev — internal testing → publishes to ppa-build-dev
  • main — next beta → publishes to ppa-build
  • release/YY.MM — point release on a maintained cycle → publishes to ppa-build-YY.MM (and to ppa-build-stable if this is the currently-promoted branch). Specify cycle: release/____

Change type (tick one):

  • Patch (fix)
  • Minor (feature)
  • Major (breaking)

The change-type label is informational and only affects how the entry is rendered in the CHANGELOG. The actual version is computed from the branch and CalVer cycle — pnpm changeset's patch/minor choice does not influence it.

Checklist

  • Changeset added — I have run pnpm changeset (or pnpm changeset --empty if no CHANGELOG line is warranted) and committed the resulting .md file. Required for PRs targeting main and release/*; enforced by the Changeset check workflow.
  • UI verified — I have verified the changes locally.
  • Linting clean — No linting errors are present (especially in scripts/).

Versioning Reminder

Important

Landscape UI uses CalVer (YY.MM.Point.Build). Version derivation by branch:

  • main / point/*YY.MM.0.<run>-beta
  • devYY.MM.0.<run>-dev
  • release/YY.MMYY.MM.1.<run> (cycle pinned by branch name)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors publications-related UI in Mirrors (and Local Repositories) to reduce duplicated API hook implementations by reusing the Publications feature’s query hook(s) and introducing a shared AssociatedPublicationsCount component.

Changes:

  • Added AssociatedPublicationsCount to centralize “publications count + link” rendering for any source name.
  • Updated Mirrors and Local Repositories lists/modals/forms to use shared publications hooks/components, removing mirrors-specific useListPublications / useListPublicationTargets hooks and the MirrorPublicationsLink component.
  • Simplified/standardized conditional rendering around publications association messaging in remove flows.

Reviewed changes

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

Show a summary per file
File Description
src/features/publications/index.ts Exports the new shared AssociatedPublicationsCount component.
src/features/publications/components/AssociatedPublicationsCount/index.ts Barrel export for the new component.
src/features/publications/components/AssociatedPublicationsCount/AssociatedPublicationsCount.tsx New shared UI for showing linked publication counts for a given source.
src/features/publications/components/AssociatedPublicationsCount/AssociatedPublicationsCount.test.tsx Tests for the new shared publication-count component.
src/features/mirrors/components/RemoveMirrorModal/RemoveMirrorModal.tsx Uses shared publications hook and adds loading-state handling while fetching publications.
src/features/mirrors/components/PublishMirrorForm/PublishMirrorForm.tsx Replaces mirrors-local publication/target listing with feature hooks; updates publish destination toggle UI.
src/features/mirrors/components/MirrorsList/MirrorsList.tsx Replaces mirrors-specific publications link with shared AssociatedPublicationsCount.
src/features/mirrors/components/MirrorPublicationsLink/MirrorPublicationsLinks.tsx Removes mirrors-specific publications link component (deduplication).
src/features/mirrors/components/MirrorPublicationsLink/MirrorPublicationsLink.test.tsx Removes tests for deleted MirrorPublicationsLink.
src/features/mirrors/components/MirrorPublicationsLink/index.ts Removes barrel export for deleted MirrorPublicationsLink.
src/features/mirrors/components/MirrorDetails/MirrorDetails.tsx Uses shared publication targets hook; adjusts publish action enable/disable behavior.
src/features/mirrors/components/MirrorActions/MirrorActions.tsx Uses shared publication targets hook; adjusts publish action enable/disable behavior.
src/features/mirrors/api/useListPublicationTargets.ts Removes duplicated mirrors-local publication-target listing hook.
src/features/mirrors/api/useListPublications.ts Removes duplicated mirrors-local publications listing hook.
src/features/mirrors/api/index.ts Stops exporting removed mirrors-local hooks.
src/features/local-repositories/components/RemoveLocalRepositoryModal/RemoveLocalRepositoryModal.tsx Minor content formatting change (removes <br />).
src/features/local-repositories/components/LocalRepositoriesList/LocalRepositoriesList.tsx Uses shared AssociatedPublicationsCount in the publications column.
src/features/local-repositories/components/LocalRepositoriesList/components/LocalRepositoryPublicationsCount/LocalRepositoryPublicationsCount.test.tsx Removes tests for deleted local-repo-specific publications-count component.
src/features/local-repositories/components/LocalRepositoriesList/components/LocalRepositoryPublicationsCount/index.ts Removes barrel export for deleted component.
.changeset/lucky-donkeys-kneel.md Adds an empty changeset entry for the main-targeting PR.
Comments suppressed due to low confidence (2)

src/features/publications/components/AssociatedPublicationsCount/AssociatedPublicationsCount.tsx:16

  • AssociatedPublicationsCount fetches all publication pages via useGetPublicationsBySource (which loops over nextPageToken). Rendering this component in lists (mirrors/local repositories) can cause N+1 network traffic and long blocking spinners for sources with many publications. Consider a dedicated hook for counts that only fetches the first page and shows a limited count (e.g. 3+) when nextPageToken is present, or otherwise avoid per-row full pagination.
    src/features/mirrors/components/PublishMirrorForm/PublishMirrorForm.tsx:60
  • If publicationTargets is empty, the "New publication" path renders PublishMirrorNewForm with no target options, which can leave the form in an unusable state (required select with no options). Guard the render so the new form is only shown when targets exist (and fall back to the existing-publication form when possible).
        {useNewPublication ? (
          <PublishMirrorNewForm
            mirror={mirror}
            publicationTargets={publicationTargets}
          />
        ) : (
          <PublishMirrorExistingForm
            mirror={mirror}
            publications={publications}
          />
        )}

Comment thread src/features/mirrors/components/RemoveMirrorModal/RemoveMirrorModal.tsx Outdated
marqode
marqode previously approved these changes Jun 30, 2026

@marqode marqode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, one suggestion and a comment on what may be a slight regression in error reporting.

Comment thread src/features/mirrors/components/MirrorsList/MirrorsList.tsx
@gesquivelgaghi gesquivelgaghi requested a review from a team July 1, 2026 13:05

@marqode marqode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@gesquivelgaghi gesquivelgaghi requested a review from a team July 1, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants