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

Show bulk upload source on entity detail page #949

Merged
merged 6 commits into from
Mar 5, 2024
Merged

Conversation

ktuite
Copy link
Member

@ktuite ktuite commented Feb 28, 2024

Closes getodk/central#583

Activity Feed

Entity create event from bulk upload:
Screenshot 2024-02-28 at 10 58 13 AM

I wanted to point out that an entity created from a submission looks like this:
Screenshot 2024-02-28 at 10 58 19 AM

In the submission case, there really is a separate source event to show. In the bulk entity upload case, it is literally the same event. We get the submission event off the entity create event and artificially insert it into the activity feed.

We could pretend there are two events, but I wasn't sure how the one event should be broken down or if it was even worth it.

Possibility (show size, too):
Screenshot 2024-02-28 at 10 55 33 AM

Conflict Tables

Screenshot 2024-02-28 at 11 31 47 AM Screenshot 2024-02-28 at 11 34 44 AM

Basic Information

Screenshot 2024-02-28 at 1 15 26 PM

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

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

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?

Does this change require updates to user documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

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

@ktuite
Copy link
Member Author

ktuite commented Feb 28, 2024

(note to self) If we did want to show the file/upload size, here are some of my code snippets:

<template #size>
  <span class="source-size">({{ entry.details.source.size / 1000 }}Kb)</span>
</template>
  .source-size {
    font-weight: normal;
    font-family: $font-family-monospace;
    font-size: 12px;
    color: #777;
  }
"bulkSource": "File {name} {size} uploaded by {actor}"

@ktuite ktuite linked an issue Feb 28, 2024 that may be closed by this pull request
@ktuite ktuite marked this pull request as ready for review February 29, 2024 22:14
@ktuite ktuite requested a review from matthew-white February 29, 2024 22:14
Copy link
Member

Choose a reason for hiding this comment

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

If you click the link, are you scrolled to the entity.bulk.create event in the activity feed? I would think that scrollData() would have to change in EntityActivity. We know that the entity.bulk.create event always corresponds to the first version of the entity, so hopefully it'd be the same logic as for an entity.create event.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not going to add a test for this because I don't see a test for the entity.create case, but I did manually try it.

if (submission.value !== undefined || !audits.dataExists) return;
const audit = audits.find(({ action }) => action === 'entity.create');
// if we don't have the audit data yet, or if we have already found the source, return.
if ((!audits.dataExists || source.value !== undefined)) return;
Copy link
Member

Choose a reason for hiding this comment

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

I might be completely misremembering, but isn't information about the source available on the entity response these days? It'd be pretty cool not to have to use audits and to get rid of this watchEffect() at some point in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's available on the versions response but not the main entity response yet!

@@ -224,7 +253,8 @@ const versionAnchor = (v) => `#v${v}`;
"submission": "Created Entity {label} in {dataset} Entity List",
// This text is shown in a list of events. {name} is the name of a Web
// User.
"api": "Entity {label} created by {name}"
"api": "Entity {label} created by {name}",
"bulkSource": "File {name} uploaded by {actor}"
Copy link
Member

Choose a reason for hiding this comment

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

At first, I was thinking that we should use @transifexKey so that this message could be reused in EntityVersionLink. But thinking about it more, the two messages appear in different contexts that I could see affecting the translation. This is more like a sentence describing an event, while the message in EntityVersionLink is more like a noun phrase. That said, I do think it'd be helpful to add a comment here for translators.

Copy link
Member

Choose a reason for hiding this comment

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

Possibly related: I was looking at the release criteria, and I noticed that the text suggested for EntityVersionLink is subtly different from the text here (the same as the text in EntityVersionLink in this PR). In the release criteria, it's "upload", not "uploaded". I actually think "uploaded" reads better in EntityVersionLink than "upload", so I like it the way it is. But maybe the difference is getting at the different contexts in which the messages appear?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, yeah, I see the distinction. I'll set the EntityVersionLink back to just "upload" for now just to match the release criteria, but we could tweak these things!

@ktuite ktuite merged commit b69839d into master Mar 5, 2024
1 check passed
@ktuite ktuite deleted the ktuite/bulk_source branch March 5, 2024 23:46
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.

See upload as the origin of an Entity
2 participants