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

[conflict_resolver] Add description field #8845

Merged

Conversation

charlottesce
Copy link
Contributor

@charlottesce charlottesce commented Jul 19, 2023

Brief summary of changes

This adds a 'Description' field to the Conflict Resolver module (both Resolved and Unresolved tabs), which corresponds to the description of the 'Question' with the conflict. Instead of using parameter_type like in the CCNA code, it uses /dictionary/module/instruments to get the description of the fields. Since some fields don't have descriptions (e.g. some _status variables), the empty string is used as default.

Changes have been made to the Instrument_LINST file such that numeric and date elements are added to the dictionary, even if they are not in the top page (inspired by the change in #8869).

Testing instructions (if applicable)

  1. Go to Clinical -> Conflict Resolver
  2. Ensure both tabs 'Unresolved' and 'Resolved' load correctly, and in both, ensure that:
    a. You can see a 'Description' selection filter and the field in the table with conflicts
    b. You can input some value in the 'Description' selection filter (e.g. 'date') and that conflicts are filtered appropriately
  3. Make sure the rest of the module works as expected according to the test plan

Note

This is a CCNA override - https://github.com/aces/CCNA/pull/6138

Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

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

All good.
And it works just fine :)

@driusan driusan added the State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed label Aug 8, 2023
@charlottesce charlottesce added the State: Needs work PR awaiting additional work by the author to proceed label Aug 10, 2023
@charlottesce charlottesce changed the base branch from main to 24.1-release August 21, 2023 14:27
@charlottesce charlottesce changed the base branch from 24.1-release to main August 21, 2023 14:27
@charlottesce charlottesce removed State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed State: Needs work PR awaiting additional work by the author to proceed labels Aug 21, 2023
Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'll wait until @xlecours re-reviews before merging since it's changed significantly since his approval..

Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

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

Splendide :)

@driusan driusan merged commit b0a24a9 into aces:main Aug 29, 2023
19 checks passed
@ridz1208 ridz1208 added this to the 26.0.0 milestone Nov 9, 2023
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.

4 participants