Skip to content

chore: move QuickSight embedding behind a workspace package#1693

Merged
paulfalgout merged 3 commits into
developfrom
quicksight-pkg
May 20, 2026
Merged

chore: move QuickSight embedding behind a workspace package#1693
paulfalgout merged 3 commits into
developfrom
quicksight-pkg

Conversation

@nmajor25
Copy link
Copy Markdown
Contributor

@nmajor25 nmajor25 commented May 18, 2026

Closes FE-131


Summary by cubic

Moved AWS QuickSight embedding into a new workspace package @roundingwell/care-ops-quicksight to centralize the SDK and simplify the dashboards app. Addresses FE-131 by wrapping embedding and fixing a race with a cached embedding context promise.

  • Refactors

    • Exposed getEmbeddingContext() with a memoized promise that resets on error; embedDashboard() uses it.
    • Removed SDK imports and manual context handling from dashboard_app; IframeView now calls embedDashboard directly.
  • Dependencies

    • Moved amazon-quicksight-embedding-sdk under @roundingwell/care-ops-quicksight; removed from root.
    • Added the new package to workspaces.

Written for commit 9b4b9fa. Summary will update on new commits. Review in cubic

@nmajor25 nmajor25 requested a review from paulfalgout May 18, 2026 20:07
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Won't 100% know that this works until it's out on QA.

The quicksight dashboard setup on my dev box is not valid. It 500's with or without the changes in this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

npm run deploy:qa2 or npm run deploy:qa

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just pushed it out to qa1, looks like it works as before.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 6 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/js/views/dashboards/dashboard_views.js
Comment thread packages/care-ops-quicksight/index.js Outdated
@cypress
Copy link
Copy Markdown

cypress Bot commented May 18, 2026

RoundingWell Care Ops Frontend    Run #8928

Run Properties:  status check passed Passed #8928  •  git commit 9b4b9fafb4: chore(dashboards): cache embedding context promise result during initialization
Project RoundingWell Care Ops Frontend
Branch Review quicksight-pkg
Run status status check passed Passed #8928
Run duration 02m 08s
Commit git commit 9b4b9fafb4: chore(dashboards): cache embedding context promise result during initialization
Committer nmajor25
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 236
View all changes introduced in this branch ↗︎

@coveralls
Copy link
Copy Markdown

coveralls commented May 18, 2026

Coverage Report for CI Build 7895

Coverage remained the same at 99.975%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 1 of 1 lines across 1 file are fully covered (100%).
  • 1 coverage regression across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/js/views/patients/patient/flow/flow_views.js 1 99.37%

Coverage Stats

Coverage Status
Relevant Lines: 6103
Covered Lines: 6102
Line Coverage: 99.98%
Relevant Branches: 1884
Covered Branches: 1883
Branch Coverage: 99.95%
Branches in Coverage %: Yes
Coverage Strength: 295.89 hits per line

💛 - Coveralls

Comment thread package-lock.json
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure what the solution is. But when I run npm install locally on my mac machine, these are removed from the package-lock.json:

    "node_modules/@emnapi/core": {
      "version": "1.10.0",
      "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.10.0.tgz",
      "integrity": "sha512-yq6OkJ4p82CAfPl0u9mQebQHKPJkY7WrIuk205cTYnYe+k2Z8YBh11FrbRG/H6ihirqcacOgl2BIO8oyMQLeXw==",
      "dev": true,
      "license": "MIT",
      "optional": true,
      "dependencies": {
        "@emnapi/wasi-threads": "1.2.1",
        "tslib": "^2.4.0"
      }
    },
    "node_modules/@emnapi/runtime": {
      "version": "1.10.0",
      "resolved": "https://registry.npmjs.org/@emnapi/runtime/-/runtime-1.10.0.tgz",
      "integrity": "sha512-ewvYlk86xUoGI0zQRNq/mC+16R1QeDlKQy21Ki3oSYXNgLb45GV1P6A0M+/s6nyCuNDqe5VpaY84BzXGwVbwFA==",
      "dev": true,
      "license": "MIT",
      "optional": true,
      "dependencies": {
        "tslib": "^2.4.0"
      }
    },

Copy link
Copy Markdown
Contributor Author

@nmajor25 nmajor25 May 18, 2026

Choose a reason for hiding this comment

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

Which then causes the circleci npm ci install to fail. Because that is on a linux machine and needs those packages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So I had to manually add those back to the package-lock.json file to get things working on ci.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you're sure you're on node 24.15?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was using v24.12, that would cause this issue?

Copy link
Copy Markdown
Contributor Author

@nmajor25 nmajor25 May 19, 2026

Choose a reason for hiding this comment

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

After switching to v24.15, it doesn't seem to remove those on npm install. So think we're good.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/care-ops-quicksight/index.js">

<violation number="1" location="packages/care-ops-quicksight/index.js:7">
P1: Reset the cached embedding promise when initialization fails; otherwise one transient `createEmbeddingContext()` error permanently breaks all later embeds in this session.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/care-ops-quicksight/index.js Outdated
@nmajor25 nmajor25 force-pushed the quicksight-pkg branch 3 times, most recently from 2ec3da6 to a636d7b Compare May 18, 2026 20:44

import { View } from 'marionette';

import { embedDashboard } from '@roundingwell/care-ops-quicksight';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah I dunno.. I think this still might be better in the beforeStart because that was running onFail on a failure.. now there's no real failure route for the amazon sdk

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I updated this to put it back in the beforeStart 👍

Comment thread package-lock.json
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you're sure you're on node 24.15?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

npm run deploy:qa2 or npm run deploy:qa

@nmajor25 nmajor25 requested a review from paulfalgout May 19, 2026 15:08
@nmajor25 nmajor25 changed the title Move QuickSight embedding behind a workspace package chore: move QuickSight embedding behind a workspace package May 19, 2026
@paulfalgout paulfalgout merged commit 228f3af into develop May 20, 2026
5 checks passed
@paulfalgout paulfalgout deleted the quicksight-pkg branch May 20, 2026 05:45
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