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.

Sign up for GitHub

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

Jump to bottom

Ruff: clean-up after multiple merges not cover by new rules #10078

Merged
merged 15 commits into from
May 2, 2024
Merged

Ruff: clean-up after multiple merges not cover by new rules #10078

merged 15 commits into from
May 2, 2024

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented May 1, 2024 •
edited
Loading

Multiple merges in a similar time have not been able to cover new Ruff rules.
This is a cleanup of these merges.
And solution for

@github-actions github-actions bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR apiv2 unittests ui parser labels May 1, 2024
@dryrunsecurity DryRunSecurity
Copy link

dryrunsecurity bot commented May 1, 2024 •
edited
Loading

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Authn/Authz Analyzer ❕ 16 findings
Configured Codepaths Analyzer ✅ 0 findings
Sensitive Files Analyzer ❕ 1 finding
AppSec Analyzer ✅ 0 findings
Secrets Analyzer ✅ 0 findings

Note

🔴 Risk threshold exceeded. Adding a reviewer if one is configured in .dryrunsecurity.yaml.

notification list: @mtesauro @grendel513

Powered by DryRun Security

@kiblik kiblik force-pushed the ruff_post_multi_merge branch from bd7b053 to 34ba794 Compare May 1, 2024 11:39
@kiblik kiblik marked this pull request as ready for review May 1, 2024 11:40
@kiblik
Copy link
Contributor Author

kiblik commented May 1, 2024

I generated this PR without deeper analysis (just by using ruff .... --fix) but I just noticed that some of changes will be dropped soon (all create_notification will be changed in #8824) and I suppose many on them should keep % syntax and base strings should be moved to _(...).

mtesauro
mtesauro approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

cneill
cneill requested changes May 1, 2024
View reviewed changes
Copy link
Contributor

@cneill cneill left a comment

Choose a reason for hiding this comment

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

Just a couple suggestions on this one

dojo/api_v2/permissions.py Outdated Show resolved Hide resolved
dojo/api_v2/permissions.py Outdated Show resolved Hide resolved
dojo/forms.py Outdated Show resolved Hide resolved
dojo/notifications/helper.py Outdated Show resolved Hide resolved
dojo/settings/settings.dist.py Outdated Show resolved Hide resolved
dojo/settings/settings.dist.py Outdated Show resolved Hide resolved
dojo/settings/settings.dist.py Outdated Show resolved Hide resolved
dojo/settings/settings.dist.py Outdated Show resolved Hide resolved
dojo/settings/settings.dist.py Outdated Show resolved Hide resolved
dojo/settings/settings.dist.py Outdated Show resolved Hide resolved
kiblik and others added 13 commits May 1, 2024 20:46
@kiblik @cneill
Update dojo/api_v2/permissions.py
031b44c 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/api_v2/permissions.py
10bc40f 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/forms.py
fd78869 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/notifications/helper.py
ff8d82d 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
680557a 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
a215b20 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
74a9cc6 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
f6786b7 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
8264722 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
82859c7 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
8010cb9 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
56ee45c 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik @cneill
Update dojo/settings/settings.dist.py
0242a2a 
Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
@kiblik kiblik requested a review from cneill May 1, 2024 20:49
@cneill
Copy link
Contributor

cneill commented May 1, 2024

Ah shoot.. looks like we had a unit test relying on that unnecessary double apostrophe...

2024-05-01T21:01:34.0181329Z uwsgi-1  | ======================================================================
2024-05-01T21:01:34.0182046Z uwsgi-1  | FAIL: test_import_with_invalid_parameters (unittests.test_importers_importer.FlexibleImportTestAPI.test_import_with_invalid_parameters) [invalid engagement]
2024-05-01T21:01:34.0182289Z uwsgi-1  | ----------------------------------------------------------------------
2024-05-01T21:01:34.0182471Z uwsgi-1  | Traceback (most recent call last):
2024-05-01T21:01:34.0183034Z uwsgi-1  |   File "/app/unittests/test_importers_importer.py", line 305, in test_import_with_invalid_parameters
2024-05-01T21:01:34.0183541Z uwsgi-1  |     self.assertEqual(import0, ["Engagement '1254235' doesn''t exist"])
2024-05-01T21:01:34.0184055Z uwsgi-1  | AssertionError: Lists differ: ["Engagement '1254235' doesn't exist"] != ["Engagement '1254235' doesn''t exist"]
2024-05-01T21:01:34.0184158Z uwsgi-1  | 
2024-05-01T21:01:34.0184308Z uwsgi-1  | First differing element 0:
2024-05-01T21:01:34.0184483Z uwsgi-1  | "Engagement '1254235' doesn't exist"
2024-05-01T21:01:34.0184658Z uwsgi-1  | "Engagement '1254235' doesn''t exist"
2024-05-01T21:01:34.0184766Z uwsgi-1  | 
2024-05-01T21:01:34.0184951Z uwsgi-1  | - ["Engagement '1254235' doesn't exist"]
2024-05-01T21:01:34.0185138Z uwsgi-1  | + ["Engagement '1254235' doesn''t exist"]
2024-05-01T21:01:34.0185288Z uwsgi-1  | ?                              +
2024-05-01T21:01:34.0185387Z uwsgi-1  | 
2024-05-01T21:01:34.0185488Z uwsgi-1  | 
2024-05-01T21:01:34.0185735Z uwsgi-1  | ----------------------------------------------------------------------

cneill
cneill approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@cneill cneill left a comment

Choose a reason for hiding this comment

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

Thanks!

@blakeaowens blakeaowens merged commit 4a2ba3a into DefectDojo:dev May 2, 2024
123 checks passed
@kiblik kiblik deleted the ruff_post_multi_merge branch May 2, 2024 06:08
dogboat pushed a commit to dogboat/django-DefectDojo that referenced this pull request May 6, 2024
@kiblik @cneill
Ruff: clean-up after multiple merges not cover by new rules (DefectDo…
ff9ad89 
…jo#10078)

* Ruff: clean-up after multiple merges not cover by new rules

* Update dojo/api_v2/permissions.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/api_v2/permissions.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/forms.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/notifications/helper.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Update dojo/settings/settings.dist.py

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>

* Fix unittest

---------

Co-authored-by: Charles Neill <1749665+cneill@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blakeaowens blakeaowens blakeaowens approved these changes

@Maffooch Maffooch Maffooch approved these changes

@mtesauro mtesauro mtesauro approved these changes

@cneill cneill cneill approved these changes

Assignees
No one assigned
Labels
apiv2 parser settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR ui unittests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kiblik @cneill @mtesauro @Maffooch @blakeaowens