search

metrumresearchgroup / mrgvalprep

Helpers for prepping inputs to mrgvalidate
https://metrumresearchgroup.github.io/mrgvalprep
Other
1 stars 0 forks source link

BUG: unique requirement IDs #43

Closed seth127 closed 2 years ago

seth127 commented 2 years ago

We have a check that story IDs must be unique and another, if requirements are used, that requirement IDs must be unique.

However, we check that the requirement IDs must be unique after we join them against the stories. This should not be enforced, because multiple stories can refer to the same requirement. Instead, we need to check for uniqueness before joining. This is fixed in this PR.

Reprex of issue

Note that making the change to the test data in d5592a3cfb9dfa72982c3226d30adceb61968c40 causes the old code (prior to d626b928831014782269ab31d5a537758b54b564) to erroneously fail the tests with:

Error (test-read-spec-yaml.R:19:3): read_spec_yaml() supports requirements
<mrgvalprep_input_error/rlang_error/error/condition>
Error in `read_spec_yaml(stories = yaml_file("stories-1.yaml"), requirements = c(yaml_file("requirements-1.yaml"), 
    yaml_file("requirements-2.yaml")))`: Duplicate values for RequirementId found:
 - R002
Backtrace:
 1. mrgvalprep::read_spec_yaml(...)

With the change in d626b928831014782269ab31d5a537758b54b564 it correctly passes.

seth127 commented 2 years ago

@kyleam do you think we should do a patch release once this merges? Might be nice to get it into the new MPN next week.

kyleam commented 2 years ago

Sure. I take it you don't expect anything else to come through within that timeline (e.g., for new valdoc approach)?

seth127 commented 2 years ago

well, I was going to say yes, but actually we might as well include the helpers I wrote for that new approach. They are in #44 for your reviewing pleasure. I'll start a release branch off of that, so that it will be ready once we merge that.