Skip to content

Comments

Refactor grant selection table to be more informative#1314

Merged
markpatton merged 8 commits intomainfrom
1195-grant-selection
Jul 16, 2025
Merged

Refactor grant selection table to be more informative#1314
markpatton merged 8 commits intomainfrom
1195-grant-selection

Conversation

@markpatton
Copy link
Contributor

@markpatton markpatton commented Jul 8, 2025

  • Changes the grant links on the grant selection and selected grants tables in the workflow to open the grant details page in a new tab
  • Refactors the grant selection table to have start/end dates in columns separated from the project name as well as the status in a new column

I'm planning to ignore the sonar qube failure. It is related to duplicated code in tests and is captured in eclipse-pass/main#1177.

@markpatton markpatton requested review from Copilot and rpoet-jh July 14, 2025 12:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the grant selection table to separate project name, start/end dates, and status into distinct columns and updates grant links to open details in a new tab.

  • Split the combined project name + date column into projectName, startDate, endDate, and awardStatus columns in the workflow grants table.
  • Removed the now-obsolete grant-title-date-cell component and updated templates accordingly.
  • Updated acceptance tests and link templates to use the new CSS selectors and open grant detail links with target="_blank".

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/acceptance/proxy-submission-test.js Updated selectors from .projectname-date-column to .projectname-column
tests/acceptance/nih-submission-test.js Same selector updates across multiple scenarios
app/controllers/grants/detail.js Added optional chaining on queuedModel.submissions to avoid runtime errors
app/components/workflow-grants/index.js Replaced the combined title/date cell with separate columns for project name, dates, and status
app/components/workflow-grants/index.hbs Removed grantTitleDateCell, added dateCell component
app/components/submission-funding-table/index.hbs Corrected @target to target syntax
app/components/grant-title-date-cell/index.js Deleted obsolete component class
app/components/grant-title-date-cell/index.hbs Deleted obsolete template
app/components/grant-link-newtab-cell/index.hbs Corrected @target to target syntax
Comments suppressed due to low confidence (3)

tests/acceptance/nih-submission-test.js:49

  • Consider adding assertions for the new Start, End, and Status columns to ensure those values render correctly in acceptance tests.
    await waitFor('[data-test-grants-selection-table] tbody tr td.projectname-column');

app/components/workflow-grants/index.js:49

  • [nitpick] Using the same date-column class for both start and end dates could lead to ambiguity; consider more specific names like start-date-column and end-date-column.
      className: 'date-column',

@rpoet-jh
Copy link
Contributor

@markpatton Changes look good in general. I agree with copilot that adding rel="noopener noreferrer" would be good.

I also noticed something in my testing. If I click a grant so it is selected, then click on the award id link in the bottom table, the new tab opens fine, but if I go back to the submission workflow, the grant is deselected and removed from the top table. I guess it is acting as though the user is saying to remove the grant, but it is odd from a UX perspective. Not sure how involved the change would be, buy maybe clicking on the award id link in the bottom table should not add/remove the grant, it should just open the new tab. WDYT?

@markpatton
Copy link
Contributor Author

@rpoet-jh Both points make sense. I will take a look.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
33.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@markpatton
Copy link
Contributor Author

@rpoet-jh Fixed the selection when clicking on a grant.

Copy link
Contributor

@rpoet-jh rpoet-jh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, Mark!

@markpatton markpatton merged commit d40b8a1 into main Jul 16, 2025
4 of 5 checks passed
@markpatton markpatton deleted the 1195-grant-selection branch July 16, 2025 15:07
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.

2 participants