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

501 copy component fails when selecting a newer srg version and a control has been previously deleted in the source component #503

Conversation

vanessuniq
Copy link
Contributor

Fixed bug #501
Refactored the logic of Component#duplicate and Component#from_mapping to correctly copy a component and update the rules based a given new srg:

  • Fixed the incorrect use of ActiveRecord methods where & pluck:
    • where & pluck applies on the database level. So if the model instance the method is being called on is not persisted, the return value is an empty list. Switched to use enumerator methods select and map instead to get the expected behavior.
  • Updated the based_on attribute of the new (copied) component to be the new given srg
  • Fixed #from_mapping method to correctly check if a new list of rule versions was provided for import or if a null value was given
  • Fixed how new Rule#rule_id are generated when importing new rules to always generate unique rule_id for a rule in a given component: rule_id was generated based on the (total count of rules + 1) of the component. This created an error due the uniqueness constraint of rule_id in a given component in the scenario when some rules have been deleted from the component. Fixed it to generate new rule_id based on the largest_rule_id in the given component.
    e.g. scenario:
    1. Take component with rules ['000001', '000002', '000003']
    2. Delete the rule with rule_id '000001' from the component
    3. Now the component has rules ['000002', '000003'] total count = 2
    4. Adding a new rule to the component:
      * Generating new rule_id based on total count + 1 will generate '000003' which already exist => causing db error
      * Generating new rule_id based on largest_rule_id + 1 will generate '000004'

…g to correctly copy a component and update the rules based a given new srg:

  * Fixed the incorrect use of ActiveRecord methods where & pluck:
    - where & pluck applies on the database level. So if the model instance the      method is being called on is not persisted, the return value is an empty list. Switched to use enumerator methods select and map instead to get the expected behavior.
  * Updated the based_on attribute of the new (copied) component to be the new given srg
  * Fixed #from_mapping method to correctly check if a new list of rule versions was provided for import or if a null value was given
  * Fixed how new Rule#rule_id are generated when importing new rules to always generate unique rule_id for a rule in a given component: rule_id was generated based on the (total count of rules + 1)  of the component. This created an error due the uniqueness constraint of rule_id in a given component in the scenario when some rules have been deleted from the component. Fixed it to generate new rule_id based on the largest_rule_id in the given component.
    e.g. scenario:
        1) take component with rules ['000001', '000002', '000003']
         2) delete the rule with rule_id '000001' from the component
        3) now the component  has rules ['000002', '000003'] total count = 2
        4) adding a new rule to the component:
            - generating new rule_id based on total count + 1 will generate '000003' which already exist => causing db error
            - generating new rule_id based on largest_rule_id + 1 will generate '000004'

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>
Signed-off-by: Vanessa Fotso <vfotso@mitre.org>
@vanessuniq vanessuniq added the bug Something isn't working label Dec 6, 2022
@vanessuniq vanessuniq added this to the v3.0 milestone Dec 6, 2022
@vanessuniq vanessuniq self-assigned this Dec 6, 2022
@vanessuniq vanessuniq temporarily deployed to vulcan-pr-503 December 6, 2022 07:47 Inactive
Copy link
Contributor

@freddyfeelgood freddyfeelgood 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 in our tests.

@vanessuniq vanessuniq merged commit d193be3 into master Dec 6, 2022
@vanessuniq vanessuniq deleted the 501-copy-component-fails-when-selecting-a-newer-srg-version-and-a-control-has-been-previously-deleted-in-the-source-component branch December 6, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Merged
3 participants