Share one SyncModal between the repository sync actions - #363
Merged
Merged
Conversation
ansible-repository-sync and file-repository-sync each carried a byte identical copy of the modal, some ninety lines apiece, so the mirror and optimize wording had to be changed in two places. Moved to src/components/sync-modal.tsx as it stood; the only edit is the inline prop type becoming an IProps interface, as the other components declare theirs. No behaviour change. Each action keeps its own permission check, its own disabled reasons and its own sync call. Refs pulp#347 Assisted By: Cursor (Claude Opus 5)
Redtigercod4
approved these changes
Sep 17, 2026
Redtigercod4
left a comment
Redtigercod4
left a comment
Contributor
There was a problem hiding this comment.
Thanks for doing this work. More than happy for you to apply the identified change in this branch.
ggainey
approved these changes
Sep 17, 2026
warisshaikh1
added a commit
to warisshaikh1/pulp-ui
that referenced
this pull request
Sep 18, 2026
pulp#363 has landed, so drop the copy this branch was carrying and use the shared component. The deb sync action goes from 154 lines to 64. The one thing deb needs that ansible and file do not is the mirror switch starting off, so the component takes a defaultMirror prop that defaults to what the other two have always sent. Their behaviour is unchanged. A boolean rather than a params object deliberately: the effect that resets the switches has to depend on it, and an object prop would either lie about that dependency or, passed as a literal, reset the switches on every render. Refs pulp#277 Assisted By: Cursor (Claude Opus 5)
ggainey
pushed a commit
that referenced
this pull request
Sep 18, 2026
* Add deb repository and remote pages Adds a "Pulp deb" menu section with Repositories and Remotes, mirroring the structure of Pulp file: list, detail and edit for each, plus the versions and distributions tabs on a repository. Deliberately scoped to repositories and remotes. Publications are left out because deb has two publication endpoints rather than a field -- publications/deb/apt generates fresh metadata and needs a signing service, publications/deb/verbatim republishes upstream's Release byte for byte -- so a publications tab needs a design decision rather than just wiring. Content browsing is left out for the same reason: deb has a dozen content endpoints where rpm has one. RemoteForm gains the APT fields, which have no equivalent in the other plugins: distributions (suites), components, architectures, gpgkey, and the sync_sources/sync_udebs/sync_installer switches. `distributions` is added to requiredFields for deb only -- pulp_deb answers a remote without it with "This field is required.", unlike every other plugin where url alone is enough. gpgkey reuses the FileUpload treatment the certificate fields already use, since it is an armoured key file. The remaining changes are registry entries: plugin2api, the plugin unions on LazyRepositories/LazyDistributions/RepositoryForm, and the deb-only fields on the shared RemoteType. All are additive; ansible, container and file behaviour is unchanged. Refs #277 Assisted By: Cursor (Claude Opus 5) * Page through the distributions of a deb repository being deleted The lookup asked for the first hundred and left a TODO for the rest, so deleting a repository with more distributions than that would leave the excess pointing at a repository that no longer exists. Walk the pages until as many have been collected as `count` reports, stopping early on an empty page so a disagreeing count cannot loop. Refs #277 Assisted By: Cursor (Claude Opus 5) * Default a deb sync to mirror: false, as pulp_deb does Opening the sync modal offered to mirror, which deletes local content the remote no longer has. The API's own default is the non-destructive one and the modal should agree with it, so the deletion is opted into. The value was written out three times; it is now one constant, which is also where the divergence from ansible and file is explained. Refs #277 Assisted By: Cursor (Claude Opus 5) * Turn an empty deb detail lookup into a not-found rejection Both detail pages read results[0] straight out of the response, which throws on any unexpected shape, and past page-with-tabs' catch that leaves the page loading with nothing to show. Read the first result defensively instead. The comment claimed the API returns a 404; it returns 200 and an empty list, and it is this code that synthesizes the 404. Refs #277 Assisted By: Cursor (Claude Opus 5) * Collapse the deb repository breadcrumb branches Three entries tested the same tab and were filtered back out again when the answer was no. It is one decision -- which crumbs the versions tab adds -- so make it once. Refs #277 Assisted By: Cursor (Claude Opus 5) * Resolve the remote API through plugin2api Three chained ternaries picked the remote API by plugin, repeating the same list call in each branch. plugin2api already maps a plugin to its repository and distribution APIs, so add the remote API there and let the form ask for it: one switch to extend when a plugin is added, not a conditional to unpick. Refs #277 Assisted By: Cursor (Claude Opus 5) * Stop stubbing hasObjectPermission on deb distributions The stub answered true to every permission. Nothing in this list is gated on one, and an apt repository carries no my_permissions field for the ansible-style check to read, so copying that check would answer no to everyone instead. Leave the permission out and say why, so an action added later gets a real check rather than a stub that always agrees. Refs #277 Assisted By: Cursor (Claude Opus 5) * Describe the deb remote and repository as interfaces Both types hold property declarations only, with no constructor, no methods and no instantiation, which is what an interface is for. Also fixes a typo in the smartUpdate comment. Refs #277 Assisted By: Cursor (Claude Opus 5) * Add smoke tests for the deb pages Reaches both lists and their empty states, then opens the remote form to check the APT-only fields render, since those are what the shared form gained for this plugin. Refs #277 Assisted By: Cursor (Claude Opus 5) * Take the shared SyncModal for the deb sync action #363 has landed, so drop the copy this branch was carrying and use the shared component. The deb sync action goes from 154 lines to 64. The one thing deb needs that ansible and file do not is the mirror switch starting off, so the component takes a defaultMirror prop that defaults to what the other two have always sent. Their behaviour is unchanged. A boolean rather than a params object deliberately: the effect that resets the switches has to depend on it, and an object prop would either lie about that dependency or, passed as a literal, reset the switches on every render. Refs #277 Assisted By: Cursor (Claude Opus 5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Suggested by @Redtigercod4 while reviewing #347, as a precursor to it.
ansible-repository-syncandfile-repository-synceach carried a byte identical copy of the sync modal, about ninety lines apiece, so the mirror and optimize wording had to be changed in two places. This moves it tosrc/components/sync-modal.tsxas it stood -- the only edit is the inline prop type becoming anIPropsinterface, the way the other components declare theirs.No behaviour change. Both actions keep their own permission check, disabled reasons and sync call; only the modal is shared.
One thing I noticed but deliberately left alone, since this is meant to be a pure move:
file-repository-syncgates onfile.change_collectionremote, which looks like it was copied from the ansible original -- pulp_file has no collection remote, so I would expect something likefile.change_filerepositorythere. Happy to fix it here or in its own PR, whichever you prefer.Assisted By: Cursor (Claude Opus 5); reviewed by me before submitting.