Skip to content
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

Hot fix for missing api v1 reference in PlatePresenter #1959

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

sdjmchattie
Copy link
Contributor

Note: I will release from this branch as well as merging when this is approved.

Changes proposed in this pull request

  • Add the api attribute for the ExtendedCsv module.
  • Pass the attribute in to the presenter if it supports it (currently only PlatePresenter).

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
    - Check story numbers included
    - Check for debug code
    - Check version

@sdjmchattie sdjmchattie self-assigned this Sep 24, 2024
@sdjmchattie sdjmchattie changed the base branch from develop to master September 24, 2024 15:31
Copy link

codeclimate bot commented Sep 24, 2024

Code Climate has analyzed commit 39ef0e7 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 91.1% (0.0% change).

View more on Code Climate.

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.95%. Comparing base (dac2eb4) to head (c634056).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1959   +/-   ##
=======================================
  Coverage   77.95%   77.95%           
=======================================
  Files         459      459           
  Lines       17692    17695    +3     
  Branches      225      225           
=======================================
+ Hits        13791    13794    +3     
  Misses       3899     3899           
  Partials        2        2           
Flag Coverage Δ
javascript 69.69% <ø> (ø)
pull_request 77.95% <100.00%> (?)
push 77.95% <100.00%> (+<0.01%) ⬆️
ruby 91.11% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Contributor

@StephenHulme StephenHulme left a comment

Choose a reason for hiding this comment

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

I assume you've tested this or is it an attempt to fix? Is it worth a test to catch in future?

@sdjmchattie
Copy link
Contributor Author

I assume you've tested this or is it an attempt to fix? Is it worth a test to catch in future?

I have not tested this as I don't know how to hit this code. But I'm 99.9% sure this fixes it.
I was thinking about writing a test, but this is API v1 and will be going in the bin in the next few weeks. Hence the TODO.

I think getting production in a working state takes priority over fiddling with tests.

@sdjmchattie sdjmchattie merged commit 87a6858 into master Sep 24, 2024
15 checks passed
@sdjmchattie sdjmchattie deleted the hot-fix-missing-api-v1-reference branch September 24, 2024 15:46
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