concord-consortium / codap

CODAP (Common Online Data Analysis Platform)
MIT License
94 stars 38 forks source link

187738952 v3 DI Get Component #1381

Closed tealefristoe closed 1 month ago

tealefristoe commented 1 month ago

PT Story: https://www.pivotaltracker.com/story/show/187738952

This PR adds support for most get component API requests. Types that remain unsupported have yet to be implemented in v3, most notably text and guide.

A few tiles/components return more information than they did in v2:

Additionally, this PR makes TileModel derive from V2Model so tiles/components now have names, which can be useful in referencing them. That said, following v2, default tile names have limited utility:

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 96.44670% with 14 lines in your changes missing coverage. Please review.

Project coverage is 85.83%. Comparing base (181b8da) to head (375f6d0). Report is 32 commits behind head on main.

Files Patch % Lines
v3/src/components/map/map-registration.ts 81.39% 8 Missing :warning:
v3/src/models/tiles/tile-model.ts 75.00% 3 Missing :warning:
.../case-table-card-common/case-table-card-handler.ts 97.43% 1 Missing :warning:
v3/src/components/slider/slider-registration.ts 97.05% 1 Missing :warning:
...src/data-interactive/handlers/component-handler.ts 97.82% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1381 +/- ## ========================================== + Coverage 85.71% 85.83% +0.11% ========================================== Files 522 527 +5 Lines 25582 25840 +258 Branches 6966 6612 -354 ========================================== + Hits 21927 22179 +252 - Misses 3380 3506 +126 + Partials 275 155 -120 ``` | [Flag](https://app.codecov.io/gh/concord-consortium/codap/pull/1381/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | Coverage Δ | | |---|---|---| | [cypress](https://app.codecov.io/gh/concord-consortium/codap/pull/1381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `72.45% <19.38%> (-0.10%)` | :arrow_down: | | [jest](https://app.codecov.io/gh/concord-consortium/codap/pull/1381/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `53.70% <92.89%> (+0.14%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cypress[bot] commented 1 month ago



Test summary

200 0 29 0Flakiness 1


Run details

Project codap-v3
Status Passed
Commit 70fb260883
Started Aug 6, 2024 3:45 PM
Ended Aug 6, 2024 3:55 PM
Duration 09:33 💡
OS Linux Ubuntu -
Browser Multiple

View run in Cypress Cloud ➡️


Flakiness

cypress/e2e/toolbar.spec.ts Flakiness
1 codap toolbar > will open a graph

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

tealefristoe commented 1 month ago

@kswenson I got the jest tests working again on this PR so it's ready for another review.

I did make one additional change, which was to get rid of the setOptions and setCreateOrShow arguments to ComponentHandler.create(), and instead allow a custom createOrShow function and/or options object to be included in a dictionary, along with the content.

I didn't carefully check all changes made from the original PR, so I hope no problems snuck through. I think it would have been easier to manage these changes if the original PR was accepted and merged, and then the ComponentHandler registration system was introduced in a separate PR. I think these changes were all improvements, it's just difficult to tell what changed when all the changes are stacked on top of each other.