fix: filtering gerrit reporitories #1632
Conversation
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
This PR adjusts how Gerrit repositories are handled in repository filtering/selection, removing frontend normalization/deduplication logic and expanding selected Gerrit repository URLs to include an additional Gerrit query-style URL.
Changes:
- Removed
normalizeRepoNamehelper and the related normalization-based deduplication behavior in repository UI components. - Updated repository selection handling to operate on the raw repository names/URLs without normalization.
- Expanded selected repository URL list (when Gerrit is connected) to include an additional
/q/project:Gerrit URL format.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/app/components/shared/utils/helper.ts | Removes normalizeRepoName utility that previously normalized repo URLs/names. |
| frontend/app/components/modules/project/store/project.store.ts | Adds Gerrit-specific expansion of selected repository URLs to include a /q/project: variant. |
| frontend/app/components/modules/project/components/shared/header/repository-switch/repository-search.vue | Removes normalization-based mapping and switches filtering/selection grouping to use raw repo names. |
| frontend/app/components/modules/project/components/shared/header.vue | Removes normalization + dedup logic when computing selected repo display name/count. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const match = repoUrl.match(gerritUrlPattern); | ||
| if (match) { | ||
| const [, baseWithNamespace, projectPath] = match; | ||
| expandedRepos.push(`${baseWithNamespace}/q/project:${projectPath}`); |
There was a problem hiding this comment.
selectedReposValues now expands Gerrit repos by appending a derived /q/project: URL, but the construction can generate malformed URLs:
- If
repoUrlis already in/q/project:form, this will produce/q/project:q/project:.... projectPathmay contain/and should be URL-encoded (Gerrit query URLs typically expectproject:foo%2Fbaras a single path segment).
Consider guarding against already-query URLs and encoding the project path when generating the query URL.
| const match = repoUrl.match(gerritUrlPattern); | |
| if (match) { | |
| const [, baseWithNamespace, projectPath] = match; | |
| expandedRepos.push(`${baseWithNamespace}/q/project:${projectPath}`); | |
| // Skip expansion if this is already a Gerrit query URL | |
| if (repoUrl.includes('/q/project:')) { | |
| continue; | |
| } | |
| const match = repoUrl.match(gerritUrlPattern); | |
| if (match) { | |
| const [, baseWithNamespace, projectPath] = match; | |
| const encodedProjectPath = encodeURIComponent(projectPath); | |
| expandedRepos.push(`${baseWithNamespace}/q/project:${encodedProjectPath}`); |
| const selectedReposValues = computed<string[]>(() => { | ||
| const hasGerrit = project.value?.connectedPlatforms?.some((platform) => | ||
| platform.toLowerCase().includes('gerrit'), | ||
| ); | ||
| const repos = selectedRepositories.value.map((repo: ProjectRepository) => repo.url); | ||
|
|
||
| if (hasGerrit) { | ||
| // For Gerrit repos, add the query URL format alongside each repo URL | ||
| // Input: https://gerrit.<domain>/<namespace>/<project>/<repo> | ||
| // Output: https://gerrit.<domain>/<namespace>/q/project:<project>/<repo> | ||
| const gerritUrlPattern = /^(https:\/\/gerrit\.[^/]+\/[^/]+)\/(.+)$/; | ||
| const expandedRepos: string[] = []; | ||
|
|
||
| for (const repoUrl of repos) { | ||
| expandedRepos.push(repoUrl); | ||
| const match = repoUrl.match(gerritUrlPattern); | ||
| if (match) { | ||
| const [, baseWithNamespace, projectPath] = match; | ||
| expandedRepos.push(`${baseWithNamespace}/q/project:${projectPath}`); | ||
| } | ||
| } | ||
|
|
||
| return expandedRepos; | ||
| } | ||
|
|
||
| return repos; | ||
| }); |
There was a problem hiding this comment.
selectedReposValues is used for both API repo filtering and for archive-state calculations (allArchived / hasSelectedArchivedRepos). Expanding the list with derived Gerrit query URLs changes the meaning of “selected repos” and will cause allArchived to become false when the archived list only contains the canonical repo URLs (the added /q/project: URLs won’t be present). Suggestion: keep selectedReposValues as the canonical selected repo URLs and introduce a separate computed (e.g. selectedReposValuesForApi) for the expanded Gerrit list, or adjust the archive checks to use the unexpanded URLs.
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
|
@joanagmaia I've removed the additional parameter passing. |
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
![github-license-compliance[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
… remove sharp Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
In this PR
Ticket
IN-935