Skip to content

Conversation

thinknoack
Copy link
Contributor

No description provided.

@thinknoack thinknoack self-assigned this Apr 16, 2025
@thinknoack thinknoack marked this pull request as draft April 16, 2025 23:03
Copy link

codecov bot commented Apr 16, 2025

Codecov Report

Attention: Patch coverage is 40.84507% with 84 lines in your changes missing coverage. Please review.

Project coverage is 46.35%. Comparing base (de0e70b) to head (ad1428c).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...app/src/scripts/dataset/routes/dataset-default.tsx 0.00% 41 Missing ⚠️
packages/openneuro-server/src/handlers/users.ts 0.00% 31 Missing ⚠️
packages/openneuro-server/src/routes.ts 0.00% 7 Missing ⚠️
...o-app/src/scripts/dataset/comments/coral-embed.tsx 91.07% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3427      +/-   ##
==========================================
+ Coverage   46.30%   46.35%   +0.04%     
==========================================
  Files         583      583              
  Lines       40877    40956      +79     
  Branches     1335     1346      +11     
==========================================
+ Hits        18927    18984      +57     
- Misses      21763    21786      +23     
+ Partials      187      186       -1     

☔ 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.

@thinknoack
Copy link
Contributor Author

@nellh - Would you mind reviewing this again? I have made the above adjustments, I also added the admin role for Openneuro admins, connected the embed signin to the modal, and added some basic openneuro styling.

currently going to fix the lint issues and write some test coverage, as well as wait for the coral teams response to my email

Copy link
Contributor

@nellh nellh left a comment

Choose a reason for hiding this comment

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

Looks pretty good, a few minor comments.


global.fetch = vi.fn().mockResolvedValue(Promise.resolve({
json: vi.fn().mockResolvedValue({ token: "mock-sso-token" }),
}))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably use the fetch mocking library. vitest-fetch-mock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am having an issue with vitest-fetch-mock. Is this some sort of version related vite issue?

just adding import createFetchMock from 'vitest-fetch-mock'; to my test file I get

FAIL packages/openneuro-app/src/scripts/dataset/comments/__tests__/coral-embed.spec.jsx > CoralEmbed - Simple Embed.js Check (Vitest) > should check if window.Coral exists and initializeCoralEmbed is called TypeError: Cannot set property testPath of #<Object> which has only a getter

@thinknoack thinknoack removed their assignment Oct 16, 2025
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