Skip to content

fix(legend): restore legend selection after chartutils 8 upgrade - #130

Open
Ansagan Islamgali (ansaganie) wants to merge 16 commits into
microsoft:mainfrom
ansaganie:fix/chartutils-8-upgrade
Open

fix(legend): restore legend selection after chartutils 8 upgrade#130
Ansagan Islamgali (ansaganie) wants to merge 16 commits into
microsoft:mainfrom
ansaganie:fix/chartutils-8-upgrade

Conversation

@ansaganie

Copy link
Copy Markdown
Contributor
  • upgrade chartutils to 8.3.0 and align formattingutils to 6.1.2
  • rebind legend item clicks through PowerKPI's existing selection dispatcher
  • delegate log tick power-of-ten filtering to chartutils
  • add a legend rerender selection regression test

- upgrade chartutils to 8.3.0 and align formattingutils to 6.1.2
- rebind legend item clicks through PowerKPI's existing selection dispatcher
- delegate log tick power-of-ten filtering to chartutils
- add a legend rerender selection regression test
@ansaganie

Copy link
Copy Markdown
Contributor Author

Lock file is cleaned up in ca0fa20. Two entries were still pointing at the internal feed with sha1 integrity:

  • powerbi-visuals-tools@7.1.1
  • the nested powerbi-visuals-api@5.3.0 underneath it

Both resolve from registry.npmjs.org with proper sha512 now, and it stayed a 4-line diff - nothing else in the file moved. Worth doing beyond tidiness: feed URLs make npm ci unusable for anyone outside the org, and sha1 is a supply-chain downgrade.

Since these are hashes rather than something you can sanity-check by eye in review: the swap only adopts a public hash after proving the recorded sha1 equals npmjs's published dist.shasum for the same artifact, so it can't quietly repoint at a different tarball. I also re-downloaded both tarballs afterwards and hashed them locally, and the sha512 matches what is in the lock file now. npm ci is green and both build jobs pass.

I left the npm audit findings alone on purpose. They are all dev-only (karma/socket.io, webpack-dev-server) and none of them ship inside the .pbiviz; the remaining uuid -> sockjs -> webpack-dev-server chain is only reachable via audit fix --force, which wants to install powerbi-visuals-tools@1.7.2. Trading the 7.x toolchain for a transitive dev advisory didn't seem like a good deal, but happy to pick that up separately.

One thing this does not fix, and which needs someone with repo settings: normalize-package-lock is red, but it never gets far enough to look at the lock file at all. It is a pull_request_target workflow, and actions/checkout now refuses to check out fork PR code in that privileged context:

Refusing to check out fork pull request code from a 'pull_request_target' workflow

So it will stay red on any fork PR regardless of what the lock file contains. Cleanest fix is probably splitting it in two: a pull_request-triggered job that only runs the checker (no secrets, no push), and keeping the auto-normalise-and-push path for same-repo branches behind a head.repo.full_name == github.repository guard. The alternative is allow-unsafe-pr-checkout: true, which does restore today's behaviour - the job only ever runs the trusted base-repo script against the fork's lock file and never executes fork code - but that is a security call I would rather not make unilaterally. Happy to open a separate PR either way.

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