Skip to content

Add auth_type to Aidbox insert #679

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

Merged
merged 44 commits into from
Jun 27, 2025

Conversation

katyasoup
Copy link
Collaborator

PULL REQUEST

Summary

Updates the Aidbox seed script to mark it as a SMART on FHIR server, so it no longer shows as auth type "none" in the table

Related Issue

Fixes #
#654

Anything else the review team should know?

Checklist

  • Descriptive Pull Request title
  • Link to relevant issues
  • Provide necessary context for design reviewers
  • Ensure test coverage is above agreed upon threshold
  • Update documentation

Copy link

linear bot commented Jun 5, 2025

Copy link
Collaborator

@robertandremitchell robertandremitchell left a comment

Choose a reason for hiding this comment

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

Screenshot 2025-06-05 at 3 52 59 PM tried a few times to do a clean build but couldn't get smart here. is that expected?

@katyasoup
Copy link
Collaborator Author

tried a few times to do a clean build but couldn't get smart here. is that expected?

@robertandremitchell hmm, is it showing up in your db correctly? that page is just pulling it from the column data, not sure why it wouldn't be showing in your browser unless it's not getting seeded properly somehow

@fzhao99
Copy link
Collaborator

fzhao99 commented Jun 10, 2025

this was working for me! fwiw, the aidbox seeding takes a few moments (and sometimes is still spinning in the background even after the UI spins up), so @robertandremitchell it's possible that you just need the seeding to finish first before checking

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.13%. Comparing base (a2f8db0) to head (429567a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   64.99%   66.13%   +1.14%     
==========================================
  Files         114      114              
  Lines        5656     5658       +2     
  Branches     1359     1360       +1     
==========================================
+ Hits         3676     3742      +66     
+ Misses       1966     1837     -129     
- Partials       14       79      +65     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@robertandremitchell robertandremitchell left a comment

Choose a reason for hiding this comment

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

thanks! working for me now

@katyasoup katyasoup changed the title Add auth_type to Aidbox insert [WIP] Add auth_type to Aidbox insert Jun 10, 2025
Copy link
Collaborator Author

@katyasoup katyasoup left a comment

Choose a reason for hiding this comment

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

Looked at this with @fzhao99 today and neither of us are quite sure why we keep seeing invalid jwt signature errors, since the same flow is working via the UI. Possibly more weirdness with jsdom and the hacky fix for TextEncoder...

Putting this on hold til folks are back from CSTE/we can review at eng sync

@katyasoup katyasoup force-pushed the kcd/que-295-display-smart-auth-for-aidbox branch from 15d1d32 to f5f7290 Compare June 16, 2025 21:42
@nickclyde nickclyde force-pushed the kcd/que-295-display-smart-auth-for-aidbox branch from a80af3b to 4c71136 Compare June 18, 2025 20:01
@nickclyde nickclyde changed the title [WIP] Add auth_type to Aidbox insert Add auth_type to Aidbox insert Jun 27, 2025
@katyasoup katyasoup merged commit 2b2d940 into main Jun 27, 2025
10 checks passed
@katyasoup katyasoup deleted the kcd/que-295-display-smart-auth-for-aidbox branch June 27, 2025 21:19
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.

5 participants