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

Combined workspace audits #778

Merged
merged 29 commits into from
Oct 23, 2024
Merged

Combined workspace audits #778

merged 29 commits into from
Oct 23, 2024

Conversation

amstilp
Copy link
Contributor

@amstilp amstilp commented Oct 15, 2024

  • Add audit classes for ConsortiumCombinedDataWorkspaces
  • Add views for auditing combined workspaces
  • Rework audit classes such that they are shared between audits for different workspace types
  • Move audit handling methods out of the views and into the audit result classes themselves
  • Add view mixins for audit and audit resolve views
  • Add a management command for running a combined workspace audit

We can reuse the audit results classes for audits of other workspace
types in the future. Split them into their own source file.
Because these audit result classes are intended to be used for multiple
differnet types of workspace auditing, we shouldn't set the class
of the "workspace" field to UploadWorkspace. Change it, and also
update the tests and templates where necessary.
Because we moved the audit results classes to their own source files,
it is much less cluttered to have both upload workspace audits
together. It also makes sense in a way, because they work together
(both audits should be run at the same time).
Add a specific template for this action button, and then use that
action button in the table. (Previously it was using the upload
workspace audit template because this one didn't exist yet.)
Also add templates, which will likely be updated as the views are
actually written.
The view was accidentally pointing the upload workspace sharing audit
template.
It should be ChangeToAdmin, not AddMember. Fix the audit and tests.
I had added this test to a different TestCase thinking it had been
missed earlier, but I was incorrect - the test in question is not
needed in the TestCase, and already exists where it is needed.
Also rename the file to have a more descriptive name - it's not
just upload workspaces anymore.
For both the Combined workspace sharing audit and the upload workspace
sharing audit. Green/black or green/red imply errors vs correctness,
but black does not.
Make it clear that the buttons are for auditing upload workspaces,
not the combined workspace.
This mixin defines a method to run an audit, and then adds the
results of that audit to the context. Use the new mixin in the views.
Note that we aren't using the mixin in the audit resolve method,
because it doesn't run audits in the same way.
Instead of having the logic in the view, move it into the audit
result classes themselves. This makes it so the view only has
to handle exceptions instead of logic.
Use the new mixin in the existing views.
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 99.48718% with 7 lines in your changes missing coverage. Please review.

Project coverage is 98.53%. Comparing base (45635a5) to head (475245d).
Report is 109 commits behind head on main.

Files with missing lines Patch % Lines
...jango/gregor_anvil/audit/upload_workspace_audit.py 99.47% 2 Missing ⚠️
gregor_django/gregor_anvil/viewmixins.py 96.42% 2 Missing ⚠️
...anvil/audit/workspace_auth_domain_audit_results.py 98.85% 1 Missing ⚠️
...gor_anvil/audit/workspace_sharing_audit_results.py 99.00% 1 Missing ⚠️
gregor_django/gregor_anvil/tests/factories.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #778      +/-   ##
==========================================
+ Coverage   97.95%   98.53%   +0.58%     
==========================================
  Files         149      162      +13     
  Lines       16040    20503    +4463     
==========================================
+ Hits        15712    20203    +4491     
+ Misses        328      300      -28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@amstilp amstilp merged commit e50af32 into main Oct 23, 2024
10 checks passed
@amstilp amstilp deleted the feature/combined-workspace-audits branch October 23, 2024 18:13
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.

1 participant