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

PXP-4319 Feat/cert audit log #738

Merged
merged 10 commits into from
Nov 21, 2019
Merged

PXP-4319 Feat/cert audit log #738

merged 10 commits into from
Nov 21, 2019

Conversation

mfshao
Copy link
Contributor

@mfshao mfshao commented Nov 19, 2019

This PR is a part of https://ctds-planx.atlassian.net/browse/PXP-4319

Get a trigger function for certificate table, to record when a user has submitted a ToU/PP cert

This PR denpends on userdatamodel PR uc-cdis/userdatamodel#67

Improvements

  • New trigger function for certificate table, to record when a user has submitted a ToU/PP cert

Dependency updates

  • Bump userdatamodel to 2.3.0

@github-actions
Copy link

github-actions bot commented Nov 19, 2019

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

@coveralls
Copy link

coveralls commented Nov 19, 2019

Pull Request Test Coverage Report for Build 8197

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 69.456%

Totals Coverage Status
Change from base Build 8188: 0.01%
Covered Lines: 5096
Relevant Lines: 7337

💛 - Coveralls

@lgtm-com
Copy link

lgtm-com bot commented Nov 19, 2019

This pull request introduces 15 alerts when merging c09b013 into c7562d1 - view on LGTM.com

new alerts:

  • 10 for Missing call to __init__ during object initialization
  • 4 for Module is imported with 'import' and 'import from'
  • 1 for Flask app is run in debug mode

Copy link
Contributor

@vpsx vpsx left a comment

Choose a reason for hiding this comment

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

also, do we know what the resources_granted and message columns are on the application table? These triggers won't record changes to those

fence/models.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Nov 21, 2019

This pull request fixes 15 alerts when merging 9ef01af into b3927b0 - view on LGTM.com

fixed alerts:

  • 10 for Missing call to __init__ during object initialization
  • 4 for Module is imported with 'import' and 'import from'
  • 1 for Flask app is run in debug mode

Copy link
Contributor

@vpsx vpsx left a comment

Choose a reason for hiding this comment

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

🎉 looks good now!

For posterity: In process_cert_audit(), joining on [OLD, NEW].application_id instead of [OLD, NEW].id (certificate id) because in the DELETE case, there is no more certificate row with OLD.id, so nothing would get added to the logs. The point of the JOIN is just to get to the User.id and User.username.

@mfshao mfshao merged commit 9d96c91 into master Nov 21, 2019
@mfshao mfshao deleted the feat/cert-audit-log branch November 21, 2019 20:50
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.

3 participants