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

feat: allows to audit the create_external_certificates method #84

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

andrey-canon
Copy link
Collaborator

Description

This allows to audit the create_external_certificate method

Testing instructions

1.Set external certificates feature
2. Save a GeneratedCertificate in the admin panel
3. check /admin/eox_audit_model/auditmodel/ table

@@ -9,6 +9,7 @@ djangorestframework-jsonapi==5.0.0
edx-drf-extensions
edx-i18n-tools
edx-opaque-keys
eox-audit-model
Copy link
Collaborator

Choose a reason for hiding this comment

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

So you prefer eox-audit now is mandatory to the plugin?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't consider that dependency was optional, I thought the current implementation was designed in that way due to the tests, probably in the future this should be mandatory since this is a plugin designed for NELC and doesn't make sense to make that optional for other installations when we only have one however that should be applied in another pr

"value": certificate_data["grade"],
"type": "percentage"
},
"metadata": getattr(settings, "EXTERNAL_CERTIFICATES_METADATA", {}),
Copy link
Collaborator

Choose a reason for hiding this comment

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

These settings are general settings. Not a tenant aware.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

settings are settings tenant awareness depends on eox-tenant

@github-actions github-actions bot added size/s and removed requirements size/m m lines label labels Aug 10, 2023
@andrey-canon andrey-canon force-pushed the and/audit_certificates_api_client branch from 9eb9057 to 818ad0b Compare August 10, 2023 19:08
@andrey-canon andrey-canon changed the base branch from and/add_prefix_to_reference to master August 10, 2023 19:10
@andrey-canon andrey-canon force-pushed the and/audit_certificates_api_client branch from 818ad0b to ede72cb Compare August 10, 2023 19:13
@johanseto johanseto self-requested a review August 10, 2023 19:32
@andrey-canon andrey-canon force-pushed the and/audit_certificates_api_client branch from ede72cb to 797c944 Compare August 10, 2023 21:40
@andrey-canon andrey-canon merged commit fd90b14 into master Aug 10, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants