-
Notifications
You must be signed in to change notification settings - Fork 63
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
Conversation
(note to self) If we did want to show the file/upload size, here are some of my code snippets:
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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}" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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!
Closes getodk/central#583
Activity Feed
Entity create event from bulk upload:

I wanted to point out that an entity created from a submission looks like this:

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):

Conflict Tables
Basic Information
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:
npm run test
andnpm run lint
and confirmed all checks still pass OR confirm CircleCI build passes