Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relationship alias coverage fixes #317

Merged
merged 6 commits into from
Nov 1, 2024

Conversation

hossenlopp
Copy link
Contributor

@hossenlopp hossenlopp commented Oct 28, 2024

Summary

Fixes highlighting of aliases in with/without relationships.

New behavior

Looks in the annotation for the localId that includes the relationship alias so it can be highlighted to match the relationship source.

Also uses the uncoverage work to properly 'not' change the highlighting in coverage when there is a node of the annotation that does not have a result due to its localId not existing in the logic.

Code changes

  • ClauseResultsHelpers.ts - Added new functionality to find the localId in the annotation that contains the source and alias of relationship clauses. This is then assigned as the alias for the source of the relationship so the alias can be highlighted as covered when the source is covered.
  • HTMLBuilder.ts - Now using the uniquified clauseReuslts results from uncoverage to highlight coverage and allow for annotation nodes that we do not care about (i.e. the ones with localId's only in annotations which we cannot account for in expressions) to not have any styling so they can inherit their parent styling.

Testing guidance

Run unit tests and regression scripts to look for changes in coverage. Some changes in coverage and highlighting are to be expected.

Copy link

github-actions bot commented Oct 28, 2024

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
85.63% (+0.11% 🔼)
2455/2867
🟡 Branches
73.41% (+0.15% 🔼)
2308/3144
🟢 Functions
87.95% (+0.2% 🔼)
438/498
🟢 Lines
85.91% (+0.08% 🔼)
2371/2760

Test suite run success

459 tests passing in 31 suites.

Report generated by 🧪jest coverage report action from 5769ba7

@hossenlopp hossenlopp force-pushed the relationship-alias-coverage-fixes branch from 3242601 to a871fc0 Compare October 28, 2024 21:07
@elsaperelli elsaperelli self-requested a review October 29, 2024 13:21
@elsaperelli elsaperelli self-assigned this Oct 29, 2024
Copy link
Contributor

@lmd59 lmd59 left a comment

Choose a reason for hiding this comment

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

Love that this fix is working out for us! I added some little comment cleanup suggestions so that you can easily batch most of them, and then a couple of other small suggestions.

We still have a couple of other - 1 and + 1's in the ClauseResultsHelpers file. Do we know if we will have any similar issues with these other alias id's? Might be out of scope for this task, but wanted to bring it up in case we need to create a follow on task.

src/helpers/ClauseResultsHelpers.ts Outdated Show resolved Hide resolved
src/helpers/ClauseResultsHelpers.ts Outdated Show resolved Hide resolved
src/helpers/ClauseResultsHelpers.ts Show resolved Hide resolved
src/helpers/ClauseResultsHelpers.ts Outdated Show resolved Hide resolved
src/helpers/ClauseResultsHelpers.ts Outdated Show resolved Hide resolved
src/helpers/ClauseResultsHelpers.ts Outdated Show resolved Hide resolved
src/helpers/ClauseResultsHelpers.ts Outdated Show resolved Hide resolved
@hossenlopp hossenlopp marked this pull request as ready for review October 31, 2024 17:04
@hossenlopp hossenlopp requested a review from lmd59 October 31, 2024 19:58
Copy link
Contributor

@lmd59 lmd59 left a comment

Choose a reason for hiding this comment

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

Nice updates!

Copy link
Contributor

@elsaperelli elsaperelli left a comment

Choose a reason for hiding this comment

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

Lgtm!

@hossenlopp hossenlopp merged commit 69dcd10 into master Nov 1, 2024
6 checks passed
@hossenlopp hossenlopp deleted the relationship-alias-coverage-fixes branch November 1, 2024 14:56
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