Add the hqmf identifier to a statement reference #25

dczulada · 2019-02-22T18:32:16Z

This is a work in progress for populating the hqmf_identifiers for the populations in a population set. suggest someone more familar with the code base review/modify accordingly.

relates to projecttacoma/cqm-models#85

Submitter:

This pull request describes why these changes were made.
Code diff has been done and been reviewed
Tests are included and test edge cases
Tests have been run locally and pass

Reviewer 1:

Name: @hossenlopp

Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
The tests appropriately test the new code, including edge cases
You have tried to break the code

Reviewer 2:

Name: @mayerm94

Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
The tests appropriately test the new code, including edge cases
You have tried to break the code

codecov-io · 2019-02-28T17:11:27Z

Codecov Report

Merging #25 into master will decrease coverage by <.01%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master      #25      +/-   ##
==========================================
- Coverage   82.36%   82.35%   -0.01%     
==========================================
  Files          69       69              
  Lines        4315     4314       -1     
==========================================
- Hits         3554     3553       -1     
  Misses        761      761

Impacted Files	Coverage Δ
...rser/cql/document_helpers/doc_population_helper.rb	`98.86% <100%> (+0.02%)`	⬆️
lib/measure-loader/hqmf_measure_loader.rb	`94.93% <93.75%> (-0.19%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82976cd...ef9897a. Read the comment docs.

hossenlopp · 2019-03-01T13:25:30Z

lib/measure-loader/hqmf_measure_loader.rb

@@ -80,21 +84,21 @@ def extract_population_set(population_hqmf_node)
          statement_ref_string = statement_ref.attr('extension')
          case cc.name
          when 'initialPopulationCriteria'
-            ps[:populations][HQMF::PopulationCriteria::IPP] = statement_ref_string
+            ps[:populations][HQMF::PopulationCriteria::IPP] = statement_ref_string + ".#{cc.at_css('id').attr('root')}"


Why not use a hash to break apart the parsed statement reference and hqmf_id instead of concatenating it on to pull it apart later in modelize_statement_ref_string.

I was just using the pattern that already existed.

I made the changes into a Hash.

hossenlopp

This PR should add to the existing tests to assert that a few of the parsed hqmf_ids are correct. Not an exhaustive test, just a spot check.

hossenlopp

Test additions look good. Just needs to be rebased and re-tested.

mayerm94

Reviewed last week, forgot to click approve. Looks good

* Add the hqmf identifier to a statement reference

…rk from master. (#39) * Fixed SDC loading to work with newer DRC measures. - Fixed issues with most data criteria getting thrown out. * Bring over dependabot nokogiri update and the simplexml_parser removal from #30. [Security] Update nokogiri requirement from ~> 1.8.5 to >= 1.8.5, < 1.11.0 Updates the requirements on [nokogiri](https://github.com/sparklemotion/nokogiri) to permit the latest version. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.8.5...v1.10.3) * Add the hqmf identifier to a statement reference (#25)

* Add the hqmf identifier to a statement reference

…rk from master. (#39) * Fixed SDC loading to work with newer DRC measures. - Fixed issues with most data criteria getting thrown out. * Bring over dependabot nokogiri update and the simplexml_parser removal from #30. [Security] Update nokogiri requirement from ~> 1.8.5 to >= 1.8.5, < 1.11.0 Updates the requirements on [nokogiri](https://github.com/sparklemotion/nokogiri) to permit the latest version. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.8.5...v1.10.3) * Add the hqmf identifier to a statement reference (#25)

* Fixed SDC loading to work with newer DRC measures. And cherry-pick work from master. (#39) * Fixed SDC loading to work with newer DRC measures. - Fixed issues with most data criteria getting thrown out. * Bring over dependabot nokogiri update and the simplexml_parser removal from #30. [Security] Update nokogiri requirement from ~> 1.8.5 to >= 1.8.5, < 1.11.0 Updates the requirements on [nokogiri](https://github.com/sparklemotion/nokogiri) to permit the latest version. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.8.5...v1.10.3) * Add the hqmf identifier to a statement reference (#25) * Port ratio/proportional cv fix from hds and add tests (#48) * codeListId and hqmfOid are both needed for sdc uniqueness * Add descriptive error message if model cannot be found * 2019 standards update (#63) * 2019 standards update entry point fix (#54) * fixed gem entry point file to be named properly * fix issue with loading api uploaded files (#55) * [Security] Bump nokogiri from 1.10.3 to 1.10.4 * Bump cqm-models version to 3.0.0 * [Security] Update rubyzip requirement from ~> 1.2.2 to >= 1.2.2, < 2.1.0 (#67) * [Security] Update rubyzip requirement from ~> 1.2.2 to >= 1.2.2, < 2.1.0 Updates the requirements on [rubyzip](https://github.com/rubyzip/rubyzip) to permit the latest version. - [Release notes](https://github.com/rubyzip/rubyzip/releases) - [Changelog](https://github.com/rubyzip/rubyzip/blob/master/Changelog.md) - [Commits](rubyzip/rubyzip@v1.2.3...v2.0.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> * Updated rubyzip dependency to be less than version 2.x, which requires ruby 2.4 * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages * Bonnie-593(ONCJira) test case fix * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages Fixed vulnerability: sparklemotion/nokogiri#1943 * BONNIE-587 Error loading VSAC value sets(ONC jira id) * Updated version of bonnie_version cqm-parser branch (#71) * Updated version of bonnie_version cqm-parser branch * Updated cqm-parser (binnie_viersion branch) * Remove unnecessary fixtures and re-include test_5_4_CQL_measure_with_drc Co-authored-by: hossenlopp <hossenlopp@mitre.org> Co-authored-by: Luke Osborne <luke.w.osborne@gmail.com> Co-authored-by: dczulada <dczulada@users.noreply.github.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Ashok Dongare <ashok.dongare@semanticbits.com>

* Fixed SDC loading to work with newer DRC measures. And cherry-pick work from master. (#39) * Fixed SDC loading to work with newer DRC measures. - Fixed issues with most data criteria getting thrown out. * Bring over dependabot nokogiri update and the simplexml_parser removal from #30. [Security] Update nokogiri requirement from ~> 1.8.5 to >= 1.8.5, < 1.11.0 Updates the requirements on [nokogiri](https://github.com/sparklemotion/nokogiri) to permit the latest version. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.8.5...v1.10.3) * Add the hqmf identifier to a statement reference (#25) * Port ratio/proportional cv fix from hds and add tests (#48) * codeListId and hqmfOid are both needed for sdc uniqueness * Add descriptive error message if model cannot be found * 2019 standards update (#63) * 2019 standards update entry point fix (#54) * fixed gem entry point file to be named properly * fix issue with loading api uploaded files (#55) * [Security] Bump nokogiri from 1.10.3 to 1.10.4 * Bump cqm-models version to 3.0.0 * [Security] Update rubyzip requirement from ~> 1.2.2 to >= 1.2.2, < 2.1.0 (#67) * [Security] Update rubyzip requirement from ~> 1.2.2 to >= 1.2.2, < 2.1.0 Updates the requirements on [rubyzip](https://github.com/rubyzip/rubyzip) to permit the latest version. - [Release notes](https://github.com/rubyzip/rubyzip/releases) - [Changelog](https://github.com/rubyzip/rubyzip/blob/master/Changelog.md) - [Commits](rubyzip/rubyzip@v1.2.3...v2.0.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> * Updated rubyzip dependency to be less than version 2.x, which requires ruby 2.4 * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages * Bonnie-593(ONCJira) test case fix * BONNIE-593 Bonnie Unresponsive Message and Error Loading Measure Packages Fixed vulnerability: sparklemotion/nokogiri#1943 * BONNIE-587 Error loading VSAC value sets(ONC jira id) * Updated version of bonnie_version cqm-parser branch (#71) * Updated version of bonnie_version cqm-parser branch * Updated cqm-parser (binnie_viersion branch) * BONNIEMAT-614 Bonnie only processing one of two measure observations(oncjira) * [Security] Bump nokogiri from 1.10.5 to 1.10.8 (#76) Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.10.5 to 1.10.8. **This update includes a security fix.** - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.10.5...v1.10.8) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> * [Security] Bump rake from 12.3.1 to 12.3.3 (#77) Bumps [rake](https://github.com/ruby/rake) from 12.3.1 to 12.3.3. **This update includes a security fix.** - [Release notes](https://github.com/ruby/rake/releases) - [Changelog](https://github.com/ruby/rake/blob/master/History.rdoc) - [Commits](ruby/rake@v12.3.1...v12.3.3) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * MAT1385_rails_upgrade_bv (#87) * made necessary upgrades to line up with bonnie's rails upgrade -> 5.2 * removed deprecated require * updated codecov (0.1.14 -> 0.2.5) and json (2.1.0 -> 2.3.1) to fix 'Title: json Gem for Ruby Unsafe Object Creation Vulnerability (additional fix)' * MAT-1308_update_cqm_models (#107) * cqm-models 3.0.0 -> 3.0.3 * ran bundle install * Mat 1708 (#112) * BONNIEMAT-623 & BONNIEMAT-629 cql-integration change * cqm-models version upgrade * [MAT-1757] Replacing VSAC Username/Password with API Key in VSAC Calls (#114) * Replacing vsac username/password with api key. * Checking for nil via safe navigation on single_code_concepts hash before checking sub-hash. When QDM datatype template has includeSubTemplate, the single_code_concepts hash will not have a related key to access the sub-hash. * Replacing safe nav operator with Hash.dig to better handle the nested hashes. * pull in cqm-model v3.0.6 version (#129) * MAT-2803 (#143) * MAT-2803 Update Bonnie to support UTF8 - QDM * Ruby 2.7.2 warnings fix * MAT-2647 Update cqm-parsers to QDM 5.6 (#148) * MAT-2647 Update cqm-parsers to QDM 5.6 * MAT-2647 Update cqm-parsers to QDM 5.6 * added git actions * removed travis.yaml * Integrate from cqm-models MAT-2993 Failed to initialize Data element with class attribute in Bonnie (#149) * ecurity vulnerabilty nokogiry (#150) * MAT-2837: Using published version of cqm-models (v4.0.0) instead of branch * bump nokogiri in case of security vulnerability * Baseline for v4.0.0 release * update Gemfile * git ignore Gemfile.lock * move ci workflow and add gitleaks.toml * address rubocop concerns * revert activesupport to support rails 5 * Bring mongoid back to 6 Co-authored-by: hossenlopp <hossenlopp@mitre.org> Co-authored-by: Luke Osborne <lwosborne@mitre.org> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Ashok <ashok.dongare@semanticbits.com> Co-authored-by: Daniel Mee <danmee10@gmail.com> Co-authored-by: Joe Kotanchik <56264529+jkotanchik-SB@users.noreply.github.com> Co-authored-by: Joe Kotanchik <joseph.kotanchik@semanticbits.com> Co-authored-by: Andrew Bird <andrew.bird@semanticbits.com>

dczulada mentioned this pull request Feb 22, 2019

A StatementReference has an HQMF identifier. projecttacoma/cqm-models#85

Merged

14 tasks

dczulada requested a review from hossenlopp February 25, 2019 13:39

hossenlopp reviewed Mar 1, 2019

View reviewed changes

dczulada force-pushed the add_population_hqmf_id branch 2 times, most recently from 7eab623 to cc463bd Compare April 3, 2019 16:32

dczulada changed the title ~~WIP Add the hqmf identifier to a statement reference~~ Add the hqmf identifier to a statement reference Apr 3, 2019

dczulada requested review from mayerm94 and hossenlopp April 3, 2019 16:35

hossenlopp requested changes Apr 5, 2019

View reviewed changes

hossenlopp reviewed Apr 15, 2019

View reviewed changes

dczulada added 3 commits April 17, 2019 15:47

Add the hqmf identifier to a statement reference

41fc2e3

Refactor statement_ref_string

5ab9a08

add assertions to test parsing hqmf_ids from measure

ef9897a

dczulada force-pushed the add_population_hqmf_id branch from c514be3 to ef9897a Compare April 17, 2019 19:49

mayerm94 approved these changes Apr 22, 2019

View reviewed changes

hossenlopp self-requested a review April 22, 2019 12:45

hossenlopp approved these changes Apr 22, 2019

View reviewed changes

jbradl11 merged commit 8ac8a7a into master Apr 22, 2019

jbradl11 deleted the add_population_hqmf_id branch April 22, 2019 14:06

hossenlopp pushed a commit that referenced this pull request Apr 25, 2019

Add the hqmf identifier to a statement reference (#25)

dbfdcb8

* Add the hqmf identifier to a statement reference

hossenlopp mentioned this pull request Apr 25, 2019

Fixed SDC loading to work with newer DRC measures. And cherry-pick work from master. #39

Merged

12 tasks

dczulada restored the add_population_hqmf_id branch May 16, 2019 15:41

lukoktonos pushed a commit that referenced this pull request Jun 27, 2019

Add the hqmf identifier to a statement reference (#25)

9a35061

* Add the hqmf identifier to a statement reference

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the hqmf identifier to a statement reference #25

Add the hqmf identifier to a statement reference #25

dczulada commented Feb 22, 2019 •

edited by hossenlopp

Loading

codecov-io commented Feb 28, 2019 •

edited

Loading

hossenlopp Mar 1, 2019

dczulada Mar 13, 2019

dczulada Apr 3, 2019

hossenlopp left a comment

hossenlopp left a comment

mayerm94 left a comment

Add the hqmf identifier to a statement reference #25

Add the hqmf identifier to a statement reference #25

Conversation

dczulada commented Feb 22, 2019 • edited by hossenlopp Loading

codecov-io commented Feb 28, 2019 • edited Loading

Codecov Report

hossenlopp Mar 1, 2019

Choose a reason for hiding this comment

dczulada Mar 13, 2019

Choose a reason for hiding this comment

dczulada Apr 3, 2019

Choose a reason for hiding this comment

hossenlopp left a comment

Choose a reason for hiding this comment

hossenlopp left a comment

Choose a reason for hiding this comment

mayerm94 left a comment

Choose a reason for hiding this comment

dczulada commented Feb 22, 2019 •

edited by hossenlopp

Loading

codecov-io commented Feb 28, 2019 •

edited

Loading