Add GitLab API base URL setting (Self-hosted)#631
Open
PhilixTheExplorer wants to merge 6 commits into
Open
Conversation
Contributor
Reviewer's GuideAdds a configurable GitLab API base URL for GitLab/self-hosted users, wires it through popup UI and storage, normalizes and validates it, and introduces dynamic optional host-permission handling plus small GitLab helper and styling tweaks. Sequence diagram for GitLab API base URL handling on report generationsequenceDiagram
actor User
participant PopupUI as popup.js
participant Storage as browser.storage.local
participant Permissions as browser.permissions
participant GitLabHelper as window.GitLabHelper
User ->> PopupUI: click generateBtn
PopupUI ->> PopupUI: normalizeGitLabBaseUrl(gitlabBaseUrlInput.value)
PopupUI ->> PopupUI: platform = platformSelect.value
alt [platform is gitlab]
PopupUI ->> PopupUI: isValidGitLabApiBaseUrl(gitlabBaseUrl)
alt [url invalid]
PopupUI ->> PopupUI: scrumHelperToast(gitlabBaseUrlInvalid)
PopupUI -->> User: stop
else [url valid]
PopupUI ->> Permissions: requestGitLabHostPermission(gitlabBaseUrl)
alt [permission granted]
PopupUI ->> Storage: set({ platform, gitlabBaseUrl, gitlabUsername })
else [permission denied or failed]
PopupUI -->> User: stop
end
end
else [platform is not gitlab]
PopupUI ->> Storage: set({ platform, platformUsername })
end
Storage -->> PopupUI: get(['platform'])
PopupUI ->> PopupUI: updatePlatformUI(platformSelect.value)
PopupUI ->> PopupUI: generateScrumReport()
Note over GitLabHelper,Storage: gitlabBaseUrl also read in forceGitlabDataRefresh
GitLabHelper ->> Storage: chrome.storage.local.get(['gitlabBaseUrl'])
Storage -->> GitLabHelper: gitlabBaseUrl
GitLabHelper ->> GitLabHelper: new GitLabHelper(gitlabBaseUrl)
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
javascript
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The GitLab base URL normalization logic is duplicated in
popup.js,main.js, and theGitLabHelperconstructor; consider centralizing this in a small shared utility (or on the helper class) so trimming/removal of trailing slashes stays consistent in one place. - The new
mapGitLabReportItem/mapGitLabReportDatalogic inscrumHelper.jsis tightly coupled to GitLab API responses; consider moving this mapping intogitlabHelper.jsso the helper encapsulates GitLab-specific transformations andscrumHelperonly consumes a GitHub-shaped model.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GitLab base URL normalization logic is duplicated in `popup.js`, `main.js`, and the `GitLabHelper` constructor; consider centralizing this in a small shared utility (or on the helper class) so trimming/removal of trailing slashes stays consistent in one place.
- The new `mapGitLabReportItem` / `mapGitLabReportData` logic in `scrumHelper.js` is tightly coupled to GitLab API responses; consider moving this mapping into `gitlabHelper.js` so the helper encapsulates GitLab-specific transformations and `scrumHelper` only consumes a GitHub-shaped model.
## Individual Comments
### Comment 1
<location path="src/scripts/scrumHelper.js" line_range="64-73" />
<code_context>
}
}
+function mapGitLabReportItem(item, projects, type, gitlabApiBaseUrl) {
+ const project = projects.find((p) => p.id === item.project_id);
+ const repoName = project ? project.name : 'unknown';
+
+ return {
+ ...item,
+ repository_url: `${gitlabApiBaseUrl}/projects/${item.project_id}`,
+ html_url:
+ type === 'issue'
+ ? item.web_url || (project ? `${project.web_url}/-/issues/${item.iid}` : '')
+ : item.web_url || (project ? `${project.web_url}/-/merge_requests/${item.iid}` : ''),
+ number: item.iid,
+ title: item.title,
+ state: type === 'issue' && item.state === 'opened' ? 'open' : item.state,
+ project: repoName,
+ pull_request: type === 'mr',
+ };
+}
+
+function mapGitLabReportData(data, gitlabApiBaseUrl) {
+ const mappedIssues = (data.issues || []).map((issue) =>
+ mapGitLabReportItem(issue, data.projects, 'issue', gitlabApiBaseUrl),
+ );
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle missing or undefined `data.projects` to avoid runtime errors when mapping GitLab items.
`mapGitLabReportItem` calls `projects.find(...)` assuming `projects` is always defined. If `data.projects` is `undefined` or missing from the GitLab response, `mapGitLabReportData` will pass `undefined` and this will throw. Consider defaulting `projects` to an empty array (e.g., `data.projects || []` or a default parameter in `mapGitLabReportItem`) so the mapping still works when project data is absent.
</issue_to_address>
### Comment 2
<location path="src/scripts/main.js" line_range="244-246" />
<code_context>
const value = gitlabTokenElement.value;
browser.storage.local.set({ gitlabToken: value });
}
+function handleGitlabBaseUrlChange() {
+ const value = normalizeGitLabBaseUrl(gitlabBaseUrlElement.value);
+ browser.storage.local.set({ gitlabBaseUrl: value });
+}
function handleProjectNameChange() {
</code_context>
<issue_to_address>
**nitpick:** Normalize the GitLab base URL input value in the UI as well as in storage.
Currently only the stored value is normalized; the input field still shows whatever the user typed (including trailing slashes/whitespace). Consider also assigning the normalized value back to `gitlabBaseUrlElement.value` so the UI shows the canonical URL after edits.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for self-hosted GitLab by introducing a configurable GitLab API base URL in the extension settings/popup, persisting it to storage, and requesting optional host permissions only when needed for report generation.
Changes:
- Added a GitLab-only “GitLab API Base URL” field (UI + storage restore/persist + normalization/validation).
- Updated GitLab fetching/mapping to use a configurable API base URL (and include it in cache keys).
- Introduced optional host-permission requesting for custom GitLab hosts, plus dark-mode styling for the new URL input.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scripts/scrumHelper.js | Loads/stores gitlabBaseUrl, instantiates GitLabHelper with it, and maps GitLab report items using the helper’s base URL. |
| src/scripts/popup.js | Adds base URL normalization/validation and requests optional host permissions before generating GitLab reports. |
| src/scripts/main.js | Persists/restores gitlabBaseUrl for non-popup/legacy settings flow. |
| src/scripts/gitlabHelper.js | Accepts configurable API base URL, normalizes it, and namespaces cache keys by base URL. |
| src/popup.html | Adds the GitLab-only base URL input with i18n label/tooltip/placeholder. |
| src/manifests/chrome.json | Declares optional host permissions to enable requesting access to custom GitLab hosts. |
| src/manifests/firefox.json | Declares optional host permissions to enable requesting access to custom GitLab hosts. |
| src/index.css | Extends dark-mode input styling to include type="url". |
| src/_locales/en/messages.json | Adds English i18n strings for the new base URL UI and permission toasts/validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PhilixTheExplorer
force-pushed
the
gitlab-base-url-setting
branch
from
9ffe8be to
df2125a
Compare
PhilixTheExplorer
force-pushed
the
gitlab-base-url-setting
branch
from
df2125a to
1566c8f
Compare
| function handleGitlabBaseUrlChange() { | ||
| const value = normalizeGitLabBaseUrl(gitlabBaseUrlElement.value); | ||
| gitlabBaseUrlElement.value = value; | ||
| browser.storage.local.set({ gitlabBaseUrl: value }); |
Comment on lines
+456
to
+463
| try { | ||
| const url = new URL(baseUrl); | ||
| return ( | ||
| (url.protocol === 'http:' || url.protocol === 'https:') && | ||
| url.pathname.endsWith('/api/v4') && | ||
| url.search === '' && | ||
| url.hash === '' | ||
| ); |
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
normalizeGitLabBaseUrlhelper is duplicated in bothpopup.jsandmain.js; consider extracting it into a shared utility to avoid divergence and keep behavior consistent. - In
forceGitlabDataRefreshyou usechrome.storage.localwhile the rest of the code relies onbrowser.storage.local; aligning on a single storage API (or a common wrapper) will simplify cross‑browser behavior and reduce surprises.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `normalizeGitLabBaseUrl` helper is duplicated in both `popup.js` and `main.js`; consider extracting it into a shared utility to avoid divergence and keep behavior consistent.
- In `forceGitlabDataRefresh` you use `chrome.storage.local` while the rest of the code relies on `browser.storage.local`; aligning on a single storage API (or a common wrapper) will simplify cross‑browser behavior and reduce surprises.
## Individual Comments
### Comment 1
<location path="src/scripts/popup.js" line_range="449-450" />
<code_context>
window.updateGenerateButtonState = updateGenerateButtonState;
+ function normalizeGitLabBaseUrl(baseUrl) {
+ return baseUrl.trim().replace(/\/+$/, '');
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Consider de-duplicating normalizeGitLabBaseUrl into a shared utility.
This implementation is duplicated here and in main.js. Centralizing it in a shared helper or namespace will keep behavior consistent and avoid having to update multiple call sites when the normalization logic changes.
Suggested implementation:
```javascript
function normalizeGitLabBaseUrl(baseUrl) {
return GitLabUtils.normalizeGitLabBaseUrl(baseUrl);
}
```
To fully de-duplicate the implementation and centralize the behavior:
1. Introduce a shared utility (for example, `src/scripts/gitlab-utils.js`) with something like:
```js
const GitLabUtils = {
normalizeGitLabBaseUrl(baseUrl) {
return baseUrl.trim().replace(/\/+$/, '');
},
};
window.GitLabUtils = GitLabUtils;
```
2. Ensure `gitlab-utils.js` is loaded before both `main.js` and `popup.js` (e.g., via `<script>` tag order or bundler configuration).
3. Update `main.js` to replace its inline `normalizeGitLabBaseUrl` implementation with `GitLabUtils.normalizeGitLabBaseUrl(baseUrl)` so both `main.js` and `popup.js` share the same normalization logic.
</issue_to_address>
### Comment 2
<location path="src/scripts/scrumHelper.js" line_range="1954-1955" />
<code_context>
hasInjectedContent = false;
// Re-instantiate gitlabHelper to ensure a fresh instance for next API call
if (window.GitLabHelper) {
+ const items = await chrome.storage.local.get(['gitlabBaseUrl']);
+ gitlabBaseUrl = items.gitlabBaseUrl || '';
gitlabHelper = new window.GitLabHelper(gitlabBaseUrl);
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using chrome.storage here may break in environments where only browser.storage is available.
This block calls `chrome.storage.local` while nearby code uses the `browser.*` API. In Firefox or WebExtension-polyfill environments, `chrome` may be undefined and cause `forceGitlabDataRefresh` to throw before `GitLabHelper` is recreated. Please reuse the existing storage abstraction (e.g., `browser.storage.local` or a shared wrapper), or at least fall back to `browser.storage` when `chrome` is unavailable to keep this cross-browser safe.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+449
to
+450
| function normalizeGitLabBaseUrl(baseUrl) { | ||
| return baseUrl.trim().replace(/\/+$/, ''); |
Contributor
There was a problem hiding this comment.
suggestion: Consider de-duplicating normalizeGitLabBaseUrl into a shared utility.
This implementation is duplicated here and in main.js. Centralizing it in a shared helper or namespace will keep behavior consistent and avoid having to update multiple call sites when the normalization logic changes.
Suggested implementation:
function normalizeGitLabBaseUrl(baseUrl) {
return GitLabUtils.normalizeGitLabBaseUrl(baseUrl);
}To fully de-duplicate the implementation and centralize the behavior:
- Introduce a shared utility (for example,
src/scripts/gitlab-utils.js) with something like:const GitLabUtils = { normalizeGitLabBaseUrl(baseUrl) { return baseUrl.trim().replace(/\/+$/, ''); }, }; window.GitLabUtils = GitLabUtils;
- Ensure
gitlab-utils.jsis loaded before bothmain.jsandpopup.js(e.g., via<script>tag order or bundler configuration). - Update
main.jsto replace its inlinenormalizeGitLabBaseUrlimplementation withGitLabUtils.normalizeGitLabBaseUrl(baseUrl)so bothmain.jsandpopup.jsshare the same normalization logic.
Comment on lines
+1954
to
+1955
| const items = await chrome.storage.local.get(['gitlabBaseUrl']); | ||
| gitlabBaseUrl = items.gitlabBaseUrl || ''; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Using chrome.storage here may break in environments where only browser.storage is available.
This block calls chrome.storage.local while nearby code uses the browser.* API. In Firefox or WebExtension-polyfill environments, chrome may be undefined and cause forceGitlabDataRefresh to throw before GitLabHelper is recreated. Please reuse the existing storage abstraction (e.g., browser.storage.local or a shared wrapper), or at least fall back to browser.storage when chrome is unavailable to keep this cross-browser safe.
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
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.
📌 Fixes
Part of #627
📝 Summary of Changes
GitLab API Base URLfield in the popup/settings UI.gitlabBaseUrlin browser storage and restored it when the popup initializes.https://gitlab.example.com/gitlab/api/v4.📸 Screenshots / Demo (if UI-related)
Both default & self-hosted gitlab flow:
Permission request:
Access granted:
Report generation:
Invalid base URL validation:
✅ Checklist
👀 Reviewer Notes
This PR intentionally keeps the scope limited to GitLab API base URL configuration and custom host permission handling.
Summary by Sourcery
Add support for configuring a GitLab API base URL, including self-hosted instances, and wire it through the popup UI, storage, and GitLab data fetching.
New Features:
Enhancements:
Build:
Summary by Sourcery
Add configurable GitLab API base URL support, including self-hosted instances, and wire it through the popup UI, storage, permissions, and GitLab data fetching.
New Features:
Enhancements:
Build: