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

Issue 1101 similar vul endpoint #1154

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

diego-avila-358
Copy link
Contributor

@diego-avila-358 diego-avila-358 commented Aug 1, 2023

Summary

Replace this with the text of what you did/doing

Have you...

  • Linked this PR to an issue?
  • Written any automated tests?
  • Removed all debug statements?
  • Refactored your code to be maintainable?
  • Checked that Gemfile.lock and yarn.lock are't getting modified unintentionally?
  • Marked as Draft if it's unfinished?

The above things are not required, but appreciated.

- Able to complete both cwe and lessons endpoints
-Wrote tests for vulnerabilities controller
 sql in vulnerabilities.rb
 - Added tests to yml
 - Fixed SQL statement for same-cwe
- Added similarVulnerability endpoint to openapi.yml
Copy link
Collaborator

@andymeneely andymeneely left a comment

Choose a reason for hiding this comment

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

Finally got around to this! Ok let's split these out to separate endpoints so we don't have to deal with if-statements and processing options. Simplifies things I think.

config/routes.rb Outdated
@@ -4,6 +4,7 @@

get '/vulnerabilities', controller: :vulnerabilities, action: :index
get '/vulnerabilities/:id', controller: :vulnerabilities, action: :show, constraints: { id: /CVE\-\d+\-\d+/ }
get '/api/vulnerabilities/:id/:similarity', controller: :vulnerabilities, action: :similarVulnerabilities, constraints: { id: /CVE\-\d+\-\d+/, similarity: /[-A-Za-z]+/ }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's actually split this out into four endpoints - :id/samedirectory, :id/samecwe, etc.. That saves us from having a giant if-statement with different queries.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That would also mean the model would be different methods too, e.g. same_cwe and same_directory

assert_equal expected_results, vulnResults, 'SQL statement is not correct'
end


Copy link
Collaborator

Choose a reason for hiding this comment

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

Should have tests for directory, lessons, and related too.

@diego-avila-358 diego-avila-358 linked an issue Aug 7, 2023 that may be closed by this pull request
3 tasks
    - sameCWE
    - sameDirectory
    - sameLessons
    -related should be reverted back to its old issue, as it needs
    a schema change.
- routing to the separation of similarVulnerabilities was also done.
-reworked and fixed all tests.
@diego-avila-358
Copy link
Contributor Author

Related should be waited on until it is incorporated into the tables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🚧 In Progress
Development

Successfully merging this pull request may close these issues.

API: Similar vulnerabilities to this one
2 participants