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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add events for row added and row updated #1101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

come-nc
Copy link

@come-nc come-nc commented May 23, 2024

No description provided.

Signed-off-by: C么me Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc added the 3. to review Waiting for reviews label May 23, 2024
@come-nc come-nc self-assigned this May 23, 2024
@come-nc come-nc requested a review from enjeck May 23, 2024 15:55
Copy link
Contributor

@enjeck enjeck left a comment

Choose a reason for hiding this comment

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

@come-nc Why do we need these events for?

@enjeck
Copy link
Contributor

enjeck commented May 27, 2024

Ah, I guess related to Workflows?

@come-nc
Copy link
Author

come-nc commented May 27, 2024

Ah, I guess related to Workflows?

Yes!

@enjeck enjeck requested a review from blizzz May 27, 2024 12:41
@enjeck enjeck requested a review from juliushaertl June 4, 2024 08:26
}

public function getRow(): Row2 {
return $this->row;
Copy link
Member

Choose a reason for hiding this comment

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

@blizzz What's your thought on just returning the entity directly here?

I'm unsure if we shall expose the internal row entity through events, rather expose specific getters or use a serializable event as #1103 mentioned

@juliushaertl
Copy link
Member

@come-nc To evaluate what we want to pass along in the event, could you share more insights on the use case you're covering with your workflow task?

@come-nc
Copy link
Author

come-nc commented Jun 10, 2024

@come-nc To evaluate what we want to pass along in the event, could you share more insights on the use case you're covering with your workflow task?

Usecases are multiple:

  • Have a workflow react to data inserted in a table (for instance a user is created in another application when a line is added to a table).
  • Have a workflow react to a modification of a row (typically a boolean column is checked to approve a vacation request or something, triggers a chain of events).

I feel like putting the whole row content information in the event is too much because it may be a lot of data, while putting only the id allows to fetch the data from the API if needed. Not entirely sure. Maybe content but capped at a given maximum length would make sense?

The event will need to be serialized by implementing upcomming OCP\EventDispatcher\IWebhookCompatibleEvent interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants