Skip to content

Match publish forms#690

Open
gesquivelgaghi wants to merge 4 commits into
mainfrom
match-publish-forms
Open

Match publish forms#690
gesquivelgaghi wants to merge 4 commits into
mainfrom
match-publish-forms

Conversation

@gesquivelgaghi

@gesquivelgaghi gesquivelgaghi commented Jun 29, 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 updates the Publications/Mirrors publish flows so the “publish” forms behave more consistently across sources (mirrors vs local repositories), including aligning required fields and read-only behavior, and then adjusts mocks/tests to match the updated UI behavior.

Changes:

  • Extend publication “new publish” form values and validation to support mirror-specific fields (distribution, architectures), and expose a dedicated mirror validation schema.
  • Update the mirror publish forms/blocks to show/edit contents in the “new” flow and lock contents in the “existing publication” flow.
  • Update MSW mocks and several component tests to reflect new data shapes and changed UI output (counts/links/columns).

Reviewed changes

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

Show a summary per file
File Description
src/tests/mocks/publications.ts Expands publication mock data (extra publications + gpgKey) to exercise updated UI and count scenarios.
src/features/publications/types/PublishValues.ts Adds optional distribution and architectures to support mirror publish form needs.
src/features/publications/index.ts Re-exports the new mirror-specific validation schema.
src/features/publications/constants.ts Introduces VALIDATION_SCHEMA_NEW_MIRROR for mirror publication creation.
src/features/publications/components/PublicationsList/PublicationsList.test.tsx Updates assertions for sources now appearing multiple times.
src/features/publications/components/AssociatedPublicationsList/AssociatedPublicationsList.test.tsx Updates expectations for table columns and duplicate source rendering/pagination behavior.
src/features/publications/components/AddPublicationForm/constants.ts Removes “Select source type” placeholder; keeps form validation shape aligned with updated UX.
src/features/publications/components/AddPublicationForm/AddPublicationForm.tsx Defaults source type to Mirror, refactors source selection logic, and aligns signing key/distribution read-only behavior.
src/features/publications/components/AddPublicationForm/AddPublicationForm.test.tsx Updates signing-key field expectation for local repository selection.
src/features/publication-targets/components/PublicationTargetList/PublicationTargetList.test.tsx Adjusts expectations for publication counts and empty-state rendering.
src/features/mirrors/components/PublishMirrorForm/components/PublishMirrorNewForm/PublishMirrorNewForm.tsx Adds editable Distribution + selectable Architectures block and uses mirror-specific validation.
src/features/mirrors/components/PublishMirrorForm/components/PublishMirrorExistingForm/PublishMirrorExistingForm.tsx Aligns labels and passes the selected publication into the contents block.
src/features/mirrors/components/PublishMirrorForm/components/PublishMirrorContentsBlock/PublishMirrorContentsBlock.tsx Locks contents display based on the existing publication instead of the mirror.
src/features/mirrors/components/MirrorPublicationsLink/MirrorPublicationsLink.test.tsx Updates expected publication count displayed in the link.
src/features/local-repositories/components/PublishLocalRepositorySidePanel/components/PublishRepositoryExistingForm/PublishRepositoryExistingForm.tsx Minor string formatting cleanup for read-only tooltips.

Comment thread src/features/publications/constants.ts

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

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

Comment thread src/tests/mocks/publications.ts
Comment thread src/tests/mocks/publications.ts
Comment thread src/tests/mocks/publications.ts Outdated
marqode
marqode previously approved these changes Jun 29, 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, a few nit-picky comments

await waitFor(() => {
expect(screen.getAllByText("1 publication")).toHaveLength(3);
expect(screen.getByText("1 publication")).toBeInTheDocument();
expect(screen.getAllByText("2 publications")).toHaveLength(2);

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.

Same as above, if it's not too hard could be worth filtering the mock data to get these numbers instead of hardcoding them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there's a lot of cases where this is a problem and I have another PR which also changes the mocks and affects some of these, so i'll address these separately

RyanLoi98
RyanLoi98 previously approved these changes Jun 30, 2026
@gesquivelgaghi gesquivelgaghi added this pull request to the merge queue Jun 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 30, 2026
Copilot AI review requested due to automatic review settings June 30, 2026 20:57

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

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

Comment on lines +233 to +237
<ReadOnlyField
label="Signing GPG key"
value={formik.values.signingKey}
tooltipMessage="The signing key can't be changed for signature-preserving mirrors."
/>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

well this suggestion is definitely not the right thing to do, but I think it might be worth getting Dani's opinion on this tomorrow

Comment thread src/tests/mocks/publications.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants