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

Remove entity label from audit log and look up info in database #959

Merged
merged 5 commits into from
Sep 6, 2023

Conversation

ktuite
Copy link
Member

@ktuite ktuite commented Aug 23, 2023

Backend part of issue #822
Frontend PR here (Makes small change on frontend to look up label from a new place.)

In the same way we look up current information about a submission when we get audit log events for a specific entity (so we can appropriately handle deleted submissions), this does the same thing for entity information.

Previously, the details for an event like entity.create contained an entity object with uuid, dataset, and label.

label has been removed, so the audit details only contain uuid and dataset but default.

But through the getBySubmissionId query, for events about entities, details.entity contains entity data populated from the database. This object has the same structure you'd see elsewhere in the API for getting details about an entity, but with the additional dataset property that was already in the audit event details.

...
    'action': 'entity.create',
    'details': {'entity': {'createdAt': '2023-08-30T19:16:38.812Z',
                         'creatorId': 5,
                         'currentVersion': {'createdAt': '2023-08-30T19:16:38.812Z',
                                            'creatorId': 5,
                                            'current': True,
                                            'data': {'circumference_cm': '333',
                                                     'geometry': '44.296333 '
                                                                 '-75.364095 0 '
                                                                 '0',
                                                     'species': 'purpleheart'},
                                            'label': '333cm purpleheart',
                                            'userAgent': 'Enketo/6.2.2 '
                                                         'Mozilla/5.0 '
                                                         '(Macintosh; Intel '
                                                         'Mac OS X 10_15_7) '
                                                         'AppleWebKit/537.36 '
                                                         '(KHTML, like Gecko) '
                                                         'Chrome/116.0.0.0 '
                                                         'Safari/537.36'},
                         'dataset': 'trees',
                         'deletedAt': None,
                         'updatedAt': None,
                         'uuid': 'f08a4912-1c45-4dc5-a27a-36a6bb5d2638'},
              'entityDefId': 2,
              'entityId': 2,
              'submissionDefId': 2,
              'submissionId': 2},
...

If the entity has been soft deleted or purged, OR if the request is NOT EXTENDED, the details will look like this (closer to how they looked before but now without label):

'details': {'entity': {'dataset': 'trees',
                     'uuid': 'f08a4912-1c45-4dc5-a27a-36a6bb5d2638'},
          'entityDefId': 2,
          'entityId': 2,
          'submissionDefId': 2,
          'submissionId': 2},

What has been done to verify that this works as intended?

Tests.

Why is this the best possible solution? Were any other approaches considered?

Follows a similar pattern as submission events for entities. Adds more useful info.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Going to make a change on frontend so deleted entities are displayed differently and dont have broken links.

Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.

I think this kind of detail about audit log event details isn't documented.

Before submitting this PR, please make sure you have:

  • run make test-full and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code from external sources are properly credited in comments or that everything is internally sourced

@ktuite ktuite force-pushed the ktuite/removeAuditEntityLabel branch 2 times, most recently from 04d4a80 to dc005fc Compare August 31, 2023 23:02
@ktuite ktuite marked this pull request as ready for review September 1, 2023 00:11
@matthew-white
Copy link
Member

matthew-white commented Sep 1, 2023

Need to make small change on frontend to look up label from a new place.

If the entity is deleted, how about we show its UUID, similar to how we show the instance ID if a submission is deleted? Ideally, we also wouldn't render a link if the entity is deleted. I think those are nice-to-haves though: we could probably back to this later once we're thinking more about entity deletion and purging.


UPDATE: Oh I see now that you mentioned that above, sorry I missed that:

Going to make a change on frontend so deleted entities are displayed differently and dont have broken links.

Copy link
Contributor

@sadiqkhoja sadiqkhoja left a comment

Choose a reason for hiding this comment

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

I like the response of this API now

lib/model/query/audits.js Outdated Show resolved Hide resolved
test/integration/api/submissions.js Outdated Show resolved Hide resolved
@ktuite ktuite force-pushed the ktuite/removeAuditEntityLabel branch from 45f864e to 352c542 Compare September 6, 2023 00:00
@ktuite ktuite merged commit 8f1d5d8 into master Sep 6, 2023
1 check passed
@ktuite ktuite deleted the ktuite/removeAuditEntityLabel branch September 6, 2023 00: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.

3 participants