feat: show iteration rollover count in issue card#19
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @braineo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the issue card display by adding an iteration rollover count. It achieves this by fetching iteration event data from the GitLab API and presenting a clear visual indicator on the card, allowing users to quickly understand an issue's iteration history.
Highlights
- New Feature: This pull request introduces the ability to display the iteration rollover count directly on issue cards, providing a quick visual indicator of how many times an issue has been moved to a new iteration.
- API Integration: The change integrates with the GitLab API to fetch resource iteration events (
/projects/:id/issues/:iid/resource_iteration_events) to accurately determine the number of times an issue has been assigned to an iteration. - UI Component: A new helper function,
createIssueCardIterationInfo, was added to dynamically render the rollover count on the issue card using a '🔄×N' format, where N is the rollover count.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds a feature to display the iteration rollover count on issue cards in the issue board. The implementation introduces a new API call for each card to fetch iteration events. While the feature itself is a nice addition, the implementation has a significant performance drawback due to the N+1 API call pattern. I've also pointed out a minor issue with CSS classes. My main recommendation is to investigate using GitLab's GraphQL API to fetch data in batches to improve performance.
| const iterationEvents = | ||
| (await fetchGitLabData<IterationEvent[]>( | ||
| getApiUrl( | ||
| `/projects/${issue.project_id}/issues/${issue.iid}/resource_iteration_events`, | ||
| ), | ||
| )) ?? []; |
There was a problem hiding this comment.
This change introduces a new API call for each issue card rendered on the board. The enhanceIssueCard function is executed for every card, and it already makes other API calls. Adding another one exacerbates an existing N+1 query problem.
For a board with many issues, this will generate a large number of requests, which can lead to:
- Hitting GitLab API rate limits.
- Poor performance and UI lag as the browser handles many concurrent network requests.
A more scalable approach would be to use the GitLab GraphQL API to fetch data for all visible issues in a single request. You could gather all the issue IDs from the newly added cards and then fire one GraphQL query to get all the iteration events.
While a full refactor might be out of scope for this PR, this is a significant performance concern that should be addressed.
| class: | ||
| 'issue-milestone-details gl-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-inline-flex gl-max-w-15 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', |
There was a problem hiding this comment.
The class attribute string contains duplicate and conflicting utility classes:
gl-max-w-15is listed twice.- Both
gl-flexandgl-inline-flexare present.gl-inline-flexwill likely overridegl-flex, makinggl-flexredundant.
Keeping the class list clean and free of duplicates improves maintainability and prevents potential styling conflicts. I've suggested a cleaned-up version.
| class: | |
| 'issue-milestone-details gl-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-inline-flex gl-max-w-15 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', | |
| class: | |
| 'issue-milestone-details gl-inline-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to display iteration rollover counts on issue cards. It adds a new interface IterationEvent, a function createIssueCardIterationInfo to render the rollover count, and integrates this into the enhanceIssueCard mutation observer. The change also broadens the issueBoardRegex to match any /boards URL. My review highlights a minor redundancy in CSS classes, a stylistic blank line, and a significant change in the issueBoardRegex scope that warrants clarification to ensure correctness and prevent unintended side effects.
| class: | ||
| 'issue-milestone-details gl-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-inline-flex gl-max-w-15 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', |
There was a problem hiding this comment.
The class gl-max-w-15 is duplicated in this string. Removing the redundancy will make the code cleaner and easier to read.
| class: | |
| 'issue-milestone-details gl-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-inline-flex gl-max-w-15 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', | |
| class: | |
| 'issue-milestone-details gl-flex gl-gap-2 gl-mr-3 gl-inline-flex gl-max-w-15 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', |
|
/gemini summary |
Summary of ChangesThis pull request introduces a new feature to display the iteration rollover count directly on issue cards within GitLab boards. It achieves this by fetching iteration event data from the GitLab API and visually representing the number of times an issue has been rolled over to a new iteration, aiming to provide more context at a glance. Highlights
Changelog
Activity
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a feature to display the iteration rollover count on issue cards. The implementation is straightforward, adding a new function to create the UI element and fetching the necessary data from the GitLab API.
My review focuses on a few key areas:
- Performance: The main concern is the introduction of an additional API call per issue card, which could negatively impact performance on boards with many issues.
- Maintainability: I've suggested a couple of minor code quality improvements, such as removing a redundant CSS class and replacing a magic string with a constant.
Overall, the feature is a useful addition. Addressing the performance concern would be beneficial for the long-term health of the script.
| const iterationEvents = | ||
| (await fetchGitLabData<IterationEvent[]>( | ||
| getApiUrl( | ||
| `/projects/${issue.project_id}/issues/${issue.iid}/resource_iteration_events`, | ||
| ), | ||
| )) ?? []; | ||
|
|
||
| createIssueCardIterationInfo( | ||
| infoItems, | ||
| iterationEvents.filter(event => event.action === 'add').length - 1, | ||
| ); |
There was a problem hiding this comment.
This change introduces an additional API call for each issue card on the board to fetch iteration events. The enhanceIssueCard function is executed for every card, and it already makes API calls to get issue details and related merge requests. Adding another call per card can lead to significant performance issues, especially on boards with many issues. This could result in a slow UI and potentially hitting GitLab's API rate limits.
Consider strategies to mitigate this, such as:
- Caching API responses to avoid re-fetching data for the same issue.
- Exploring if there's a way to batch these requests, although the GitLab API may not support it for these specific endpoints.
While a full refactor might be out of scope for this PR, it's an important consideration for the overall health of this script.
| const inline = $('<span/>').appendTo(element); | ||
| $('<div/>', { | ||
| class: | ||
| 'issue-milestone-details gl-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-inline-flex gl-max-w-15 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', |
There was a problem hiding this comment.
The CSS class gl-max-w-15 is duplicated in the class string. Removing the duplicate will make the code cleaner and avoid any potential unexpected styling behavior.
| 'issue-milestone-details gl-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-inline-flex gl-max-w-15 gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', | |
| 'issue-milestone-details gl-flex gl-max-w-15 gl-gap-2 gl-mr-3 gl-inline-flex gl-cursor-help gl-items-center gl-align-bottom gl-text-sm gl-text-gray-500', |
|
|
||
| createIssueCardIterationInfo( | ||
| infoItems, | ||
| iterationEvents.filter(event => event.action === 'add').length - 1, |
There was a problem hiding this comment.
The string 'add' is used here to filter iteration events. This is a "magic string". It's better to define it as a constant at a higher scope (e.g., at the top of the file) to improve readability and maintainability. Using a constant makes the code self-documenting and reduces the risk of typos in other parts of the code.
For example:
const ITERATION_EVENT_ACTION_ADD = 'add';
// ... later in the code
iterationEvents.filter(event => event.action === ITERATION_EVENT_ACTION_ADD).length - 11 participant
No description provided.