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.

Sign up for GitHub

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

Jump to bottom

Warn about deprecated transitions when uploading app-settings #269

Closed
dianabarsan opened this issue Nov 28, 2019 · 2 comments · Fixed by #338
Closed

Warn about deprecated transitions when uploading app-settings #269

dianabarsan opened this issue Nov 28, 2019 · 2 comments · Fixed by #338
Assignees
@newtewt @latin-panda
Labels
Type: Improvement Make something better

Comments

@dianabarsan
Copy link
Member

There are a number of deprecated transitions in cht-core, each with it's own deprecation "version":

  • update_notifications deprecated since 3.3.0 in favor of muting
  • update_sent_forms deprecated since 3.7.0 due to the refactor of Reminders
  • generate_patient_id_on_people deprecated since 3.8.0 in favor of generate_shortcode_on_contacts

medic-conf could warn, pre-upload (?), when a deprecated transition is enabled in app_settings, depending on the version of the app the instance is running.

Questions to consider:

Should the deprecation table be hardcoded in medic-conf?
Should we have an API endpoint that returns the list of deprecated transitions?
@dianabarsan dianabarsan added the Type: Improvement Make something better label Nov 28, 2019
@dianabarsan dianabarsan mentioned this issue Nov 28, 2019
Merged
@garethbowen garethbowen added the Help wanted Good for first time contributions label Jun 12, 2020
@latin-panda latin-panda self-assigned this Jun 30, 2020
This was referenced Jul 6, 2020
Merged
Merged
@dianabarsan
Copy link
Member Author

dianabarsan commented Jul 8, 2020 •
edited
Loading

This is ready for AT.
Both cht-core and medic-conf have updates to solve this issue.
Updates to cht-core can be found on the 269-warn-deprecated-transitions branch, while the updates to medic-conf can be found on the 269-log-warning-deprecated-transitions branch.

@newtewt newtewt self-assigned this Sep 8, 2020
@newtewt
Copy link
Contributor

newtewt commented Sep 9, 2020

@latin-panda I think this looks good. I wonder if this would be worth adding a check if that transition actually exists instead of causing all transitions to be disabled but is probably a separate issue.

Confirmed warning logs in apps about deprecation
Confirmed warning logs in medic-conf while uploading settings,
Confirmed medic-conf works with old versions
Confirmed cht-core works with old version of medic-conf.
Confirmed transitions still work.

@latin-panda latin-panda linked a pull request Sep 14, 2020 that will close this issue
269 - Log warning for deprecated transitions #338
Merged
latin-panda added a commit to medic/cht-core that referenced this issue Sep 14, 2020
@latin-panda
269 - Log warning for deprecated transitions (#6524)
55517be 
Ticket: medic/cht-conf#269
This commit will:
* Add transition name and deprecated properties to each transition, to identify which ones are deprecated and the version it got deprecated.
* Add function inside each transition to return the deprecation message.
* Add log the message when deprecated transitions are initialized.
* Add function in the transition's index to return the list of deprecated ones.
* Add endpoint in the Settings API to return the deprecated transitions.
* Fix a circular dependency between settings service and config.js
* Move the unit test of transition to test/unit/transitions directory
latin-panda added a commit that referenced this issue Sep 14, 2020
@latin-panda
269 - Log warning for deprecated transitions (#338)
395ddcb 
Ticket: #269
This commit will:
* Add a feature to log warning when deprecated transitions are activated when uploading app settings.
* Skip 404 errors when requesting deprecated messages
@garethbowen garethbowen removed the Help wanted Good for first time contributions label Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@newtewt newtewt

@latin-panda latin-panda

Labels
Type: Improvement Make something better
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

269 - Log warning for deprecated transitions medic/cht-conf
4 participants
@garethbowen @newtewt @dianabarsan @latin-panda