Skip to content

Fix issue selector functionality#2

Open
elenz97 wants to merge 7 commits intoGecka-Apps:mainfrom
dgrieser:fix/issue-selector
Open

Fix issue selector functionality#2
elenz97 wants to merge 7 commits intoGecka-Apps:mainfrom
dgrieser:fix/issue-selector

Conversation

@elenz97
Copy link
Contributor

@elenz97 elenz97 commented Jan 15, 2026

The issue selector was not properly filtering or displaying available issues.
This commit fixes the selector behavior to correctly show and filter GitLab issues based on user input.

@elenz97 elenz97 changed the title fix issue selector Fix issue selector functionality Jan 15, 2026
…UI building

- switched all double‑quoted strings to single quotes for consistency
- aligned import statements and removed unnecessary spacing
- condensed UI construction code with clearer layout definitions
- refactored project and issue loading logic for better readability
- added inline comments and minor variable renames without altering functionality
- updated button definitions and loading indicator handling to match new style conventions
@dgrieser
Copy link

Include avatarLoader.js in extra sources for the extension build

- Notify users and display a loading message when GitLab API returns 401 or 403, guiding them to set the server URL and token.
- Clear and refresh project and issue lists in the selector and report dialogs to reflect the missing configuration.
- Extend time‑sending component to show an error notification on authentication failures.
@loxK
Copy link
Member

loxK commented Feb 5, 2026

Thanks for the contribution! The improved error handling for 401/403 auth errors is a good addition, and catching the missing avatarLoader.js in the Makefile is helpful.

However, there are a few issues that conflict with the GNOME extension review guidelines I just addressed:

1. Use console.debug() instead of log()

The GNOME review explicitly requires using console.* methods instead of log(). From the Port Guide 45:

// ❌ Don't use
log(`GitLab Issue Selector: Fetching projects from URL: ${url}`);

// ✅ Use instead
console.debug(`GitLab Issue Selector: Fetching projects from URL: ${url}`);

2. Avoid unnecessary try-catch blocks

The review feedback asked to avoid wrapping code in try-catch when not needed. The outer try-catch added in _loadProjects() isn't necessary.

3. Remove sensitive logging

These lines could expose sensitive information:

log(`GitLab Issue Selector: Using token length: ${token ? token.length : 0}`);
log(`GitLab Issue Selector: Response body: ${response}`);

I'd suggest removing token-related logs and full response body logging entirely.

4. Reduce log verbosity

The amount of logging is quite verbose for production code. Consider keeping only essential debug statements.


Summary of requested changes:

  • Replace all log() calls with console.debug()
  • Remove the outer try-catch in _loadProjects()
  • Remove logs that expose token info or full API responses
  • Reduce overall logging verbosity

The 401/403 handling logic itself is great — just needs these adjustments to align with GNOME guidelines. Let me know if you have any questions!

@loxK
Copy link
Member

loxK commented Feb 5, 2026

Thanks for working on this! I reviewed the final diff and found some issues that need to be fixed before we can merge:

extension.js - Broken merge

  1. Duplicate code: _openReport() has the ReportDialog creation twice (lines ~367-371 and again at the end)
  2. Unwanted try-catch: GNOME review asked to remove try-catch around ReportDialog, but it's back
  3. Unnecessary _openPreferences() method: We already use this._extension.openPreferences() - this new method is not needed

issueSelector.js

  • log()console.debug() - good!
  • ❌ The outer try-catch around _loadProjects() should be removed (GNOME guidelines)

reportDialog.js

  • ✅ Looks good

Please:

  1. Fix the broken merge in extension.js
  2. Remove the outer try-catch in issueSelector.js
  3. Squash everything into 1 clean commit like fix(auth): handle 401/403 errors with user guidance

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.

3 participants