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.

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

Disable checkbox for adding collection if collection is already assigned to repository #3657

Merged
merged 1 commit into from
May 15, 2023

Conversation

jerabekjiri
Copy link
Contributor

@jerabekjiri jerabekjiri commented Apr 27, 2023

Issue: AAH-2323

User experience improvement, but for performance cost. It could be a little inefficient to fetch collections from the collection versions list again and match them with modal collection versions. So I think we need to think this through if we want to include this. :)

before:
Screenshot from 2023-04-27 13-49-31

after:
Screenshot from 2023-04-27 13-41-55

@github-actions github-actions bot added backport-4.6 This PR should be backported to stable-4.6 (2.3) backport-4.7 This PR should be backported to stable-4.7 (2.4) labels Apr 27, 2023
@jerabekjiri jerabekjiri removed the backport-4.6 This PR should be backported to stable-4.6 (2.3) label Apr 27, 2023
@himdel
Copy link
Collaborator

himdel commented Apr 27, 2023

Looks like a nice improvement 👍, but yeah, you're right the logic can get a bit ..complex :)

This will only work if the "collection version + repo" entry is in the same page of results when filtering by repo and when not, right?

But .. then again.. we're already showing "repository" in the modal list ...so is there a need for the extra query? Or could we just check that repository is not the current repository when the version matches?

(Though, if collection C version V is already in staging and validated, we should probably be greying out both entries, since that same collection version is already in our repo.)

((I wonder if we could add this as an api filter, or if it would make sense to fire a specific query to check for each C-V pair ...))

@jerabekjiri jerabekjiri force-pushed the fix/disable-cv-duplicates branch from 1e37e73 to 1159524 Compare May 2, 2023 11:34
@jerabekjiri
Copy link
Contributor Author

jerabekjiri commented May 2, 2023

This will only work if the "collection version + repo" entry is in the same page of results when filtering by repo and when not, right?

Yep, that's too bad thanks to pagination, this will only work sometimes...

But .. then again.. we're already showing "repository" in the modal list ...so is there a need for the extra query? Or could we just check that repository is not the current repository when the version matches?

make sense, checking if repository matches should be sufficient enough for now and the cleanest solution, more improvements should be made on API (remove duplicates or c-v pair match)

@jerabekjiri jerabekjiri merged commit 50f33a3 into ansible:master May 15, 2023
@patchback
Copy link

patchback bot commented May 15, 2023

Backport to stable-4.7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4.7/50f33a3a201d3240365d11203edb9009a32cc664/pr-3657

Backported as #3733

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request May 15, 2023
jerabekjiri added a commit that referenced this pull request May 16, 2023
Issue: AAH-2323
(cherry picked from commit 50f33a3)

Co-authored-by: Jiří Jeřábek (Jiri Jerabek) <Jerabekjirka@email.cz>
@github-actions github-actions bot added the backported-4.7 This PR has been backported to stable-4.7 (2.4) label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4.7 This PR should be backported to stable-4.7 (2.4) backported-4.7 This PR has been backported to stable-4.7 (2.4)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants