Skip to content

Group PRs under the issue that it closes#452

Open
IzN432 wants to merge 13 commits intoCATcher-org:mainfrom
IzN432:group-issues-under-pr
Open

Group PRs under the issue that it closes#452
IzN432 wants to merge 13 commits intoCATcher-org:mainfrom
IzN432:group-issues-under-pr

Conversation

@IzN432
Copy link
Copy Markdown
Contributor

@IzN432 IzN432 commented Mar 26, 2025

Summary:

Fixes #444

Type of change:

  • ✨ New Feature/ Enhancement

Changes Made:

  • Move PRs to below the issue that they close
    This is performed after filtering and sorting in the IssueDataTable.ts. It is implemented such that for each PR in the list, we move it to underneath the first issue that the PR closes, that does not have a PR attached to it yet.

  • Add a select form field for this behaviour in the filter

Screenshots:

WATcher change link

Proposed Commit Message:

Move PRs under the issue that they close

There is no way to visualize the relationship between PRs and issues in
WATcher.

This makes it difficult to identify anomalies such as when PRs do not
close any issues.

Let's move the PRs under the issues that they fix.

This makes it very obvious when PRs and issues are related.

Checklist:

  • I have tested my changes thoroughly.
  • I have created tests for any new code files created in this PR or provided a link to a issue/PR that addresses this.
  • I have added or modified code comments to improve code readability where necessary.
  • I have updated the project's documentation as necessary.

@IzN432
Copy link
Copy Markdown
Contributor Author

IzN432 commented Mar 26, 2025

The way this is implemented currently may cause some confusion when there are multiple issues closed by the PR or multiple PRs that close the issue. But for non anomalous usage, I think this implementation should be sufficient. Open to any suggestions if you guys have any ideas.

@Eclipse-Dominator
Copy link
Copy Markdown
Contributor

Eclipse-Dominator commented Mar 31, 2025

I've looked through the code, the current code 'groups' PR by looking at issues in the same issueDataTable. However, the nature of PRs is that the person fixing the issue might not be the one who created the issue. Thus, a more appropriate way might be to modify cardview to host a sublist of issues/prs and the content can be displayed roughly like so (in psudo code):

<for issue obj in all issues>
  <issue obj> 
  </issue obj>
  
   <a component that can be collapsed>
      <for child in issue>
         <child issue/pr></child issue/pr>
      </>
   </a component that can be collapsed>
</>

The data transformation can be done globally (ie before data is grouped in issueDataTable).

Transformation sketch

data : Issue[]

const numToIssuesMap = new Map<number, Issue>();
const issuesToPrMap = new Map<number, Issue[]>();
for (const issue of data) {
    numToIssuesMap.set(issue.id, issue);
    if (issue.issueOrPr == 'Issue') {
        issueIds.set(issue.id, []);
    }
}

for (const pr of data) {
    if (pr.issueOrPr == 'Issue') {
        continue;
    }
    for (const issueId of pr.closingIssuesReferences) {
        issueToPrMap.get(issueId)?.push(pr);
    }
}

// transform into card data

In a similar fashion, this approach can also be used to display all issues that a PR resolves on WATcher. I would suggest making them collapsible on default to prevent overcrowding the UI.

This logic can be incorporated into issue.model to minimize widespread code changes and maintain a cleaner structure.

Since this is an alternative approach to the current solution, it would be beneficial to create a separate PR to investigate and compare both approaches before deciding whether to adopt this solution.

What do you think?

@IzN432
Copy link
Copy Markdown
Contributor Author

IzN432 commented Mar 31, 2025

The issues being displayed under each assignee is the issues that are assigned to them, rather than the issues they create. In this case, PRs that close issues that are not assigned to the creator of the PR are considered anomalies, so I believe prof wants a way to identify these on glance (which would be PRs not under any issues).

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.

Show matching issues and PRs together

2 participants