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

Always upload old style and new style methodologies #47

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dormi
Copy link
Contributor

@dormi dormi commented Sep 27, 2019

Spec

In CE we exported old methodologies with version = 2
In Pro we exported new methodologies with version = 2

We cannot check the template version to know how to upload methodologies.
We can just check which methodologies come with the file to upload and upload them.

How to test

Old files (version = 2) containing new or old methodologies, must upload the methodologies correctly both in Pro and CE

process_v1_methodologies(template)

# new methodologies format
process_v2_methodologies(template)
Copy link
Contributor

Choose a reason for hiding this comment

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

In the event a methodology is imported by process_v1_methodologies(template) does process_v2_methodologies(template) essentially become a no-op?
Or vice-versa. It'll run but nothing will happen?

Copy link
Contributor Author

@dormi dormi Sep 27, 2019

Choose a reason for hiding this comment

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

Exactly, that is the idea.
The code is already there:
If no old style methodologies are present, we just return: https://github.com/dradis/dradis-projects/blob/master/lib/dradis/plugins/projects/upload/v3/template.rb#L169
If no new methodologies are present, we don't enter the loop:
https://github.com/dradis/dradis-projects/blob/master/lib/dradis/plugins/projects/upload/v3/template.rb#L130
I thought on adding an explicit return in new methodologies too. But I think it works like this.

Copy link
Contributor

@brianp brianp left a comment

Choose a reason for hiding this comment

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

I have one small question about running both versions of the code. Assuming no performance implications, and running it won't create any kind of duplication I think it's good to move to staged.

Base automatically changed from master to main January 29, 2021 10:18
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.

2 participants