feat: make card events compatible with webhook listener

[nicolashimmelmann]

• Target version: main

Summary

This PR modifies the base class of the card events to make them compatible with the webhook_listeners (docs) app, that ships with the Nextcloud server. This is needed, to be able to build flows in Nextcloud Flow / Windmill that should get triggered when a card gets created/updated/deleted in a board.

Please let me know if tests or documentation are required.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Documentation (manuals or wiki) has been updated or is not required

[elzody]

thank you for working on this. it seems logical and looks like a pretty clean and simple solution. i only have one question regarding it, and maybe it's something also for @luka-nextcloud and @grnd-alt to chime in on:

• for the lib/Event/CardUpdatedEvent.php, would this only return the serialized information for the updated card? it seems like it would not return serialized info on the card's old information as well. maybe it could then be overridden in the CardUpdatedEvent as well to return both. i think it makes sense to also give this information, what does everyone else think?

i'm also trying to think of the best way we can write tests for this. i'll see if some other apps have any reference material there for some ideas

[juliusknorr]

would this only return the serialized information for the updated card? it seems like it would not return serialized info on the card's old information as well.

Iirc @blizzz worked on that in tables. Do you remember how we did it there?

[nicolashimmelmann]

Thanks for the fast reply!

• maybe it could then be overridden in the CardUpdatedEvent as well to return both.

You are right, from my experience building a flow using my current solution, it would have been handy to have the old information as well. However, then we should document it properly that the event payload differs in this case.

[blizzz]

would this only return the serialized information for the updated card? it seems like it would not return serialized info on the card's old information as well.

Iirc @blizzz worked on that in tables. Do you remember how we did it there?

Please correct me if I mix up something.

The related PR in tables is nextcloud/tables#1101 and the specific commit is nextcloud/tables@2a6a338

I created the final, jsonserializable class OCA\Tables\Model\Public\Row (which is probably better called a Value Object or Data Transfer Object I suppose) which has the most basic Row information that is being serialized, and is otherwise decoupled from the rest of the logic. It contains both the current and the previous values.

[elzody]

@nicolashimmelmann just checking to make sure everything is all good, did you need any help getting this finished up? no pressure, just wanted to check! :)

[nicolashimmelmann]

Sorry, I think it was a misunderstanding. I assumed you would first discuss internally about the best way to implement this to match the rest of the Nextcloud architecture. But if it is fine for you, I can pursue the implementation further based on the comment from blizzz above to match it to the tables implementation.

However, in that PR there was a check if the interface exists, to be able to support NC versions < 30. Should we adopt this approach for the Deck app as well?

EDIT: Nevermind, I see it is already at 31

[elzody]

Sounds good, sorry for the confusion there! Maybe it's possible I also did not understand. :) The check would probably be useful if we want this backported to previous versions, but odds are this will be released with 31 and therefore it won't need the check. Might change in the near future, but it would be easy to add the check in that case.

@nicolashimmelmann

[nicolashimmelmann]

Hi @elzody ,

I have now continued the implementation. Before I move on, it would be great if you could give me some quick feedback :)

1. I left out the following fields compared to the DB model. Is that fine?

   • descriptionPrev: Will be replaced by a full previousVersion field in the CardUpdatedEvent
   • type: Left out since it is Marked as „for later use“ in documentation
   • attachments: Currently, no CardUpdatedEvent is fired for adding a new attachment. So no need to have them in the payload. User can call the API for attachments if needed.
   • attachmentCount: Same
   • notified: In the notification helper class it says this is used to be able to deliver due date notifications for users who have been added later to a board. So I assume this is only needed internally.

2. Also, for consistency reasons all dates are returned as ISO8601. I did not like the mix between ISO and unix timestamps… Is that fine or should I use unix timestamps everywhere?

Thank you!

[juliusknorr]

I left out the following fields compared to the DB model. Is that fine?

That sounds good 👍

Also, for consistency reasons all dates are returned as ISO8601. I did not like the mix between ISO and unix timestamps… Is that fine or should I use unix timestamps everywhere?

Yes, seems better with that.

[juliusknorr]

Code looks good to me, just CI needs to pass then this would be good from my perspective

[stefan-niedermann]

for consistency reasons all dates are returned as ISO8601. I did not like the mix between ISO and unix timestamps… Is that fine or should I use unix timestamps everywhere?

Just to be sure: Those changes do not affect the REST API model, correct?

[juliusknorr]

Just to be sure: Those changes do not affect the REST API model, correct?

Yes, this model and separate json serialization is only used for the webhooks, no changes on the API side.

[elzody]

I would try running composer run cs:fix and it will probably fix whatever formatting errors it is complaining about. For the static analysis errors, you can run composer run psalm -- --no-cache and it will tell you what is happening.

Also looks good from me, just need to get the CI passing as Julius said.

[nicolashimmelmann]

Thanks for your feedback. I will finish it soon and tag you when it's ready!