Add Item Subtypes To Event CallBacks

[khcnz]

I've found while attempting to use this library to parse webhooks events for stars added/removed that it doesn't seem possible to get the actual underlying type of "item" that the event is for (for example MessageItem)

I've prototyped how I think this could/should work - keen for your feedback before extending it o the other item subtypes and extending unit tests etc.

[stoiveyp]

Hi - thanks for reaching out on this. It's my working day right now and then I'm travelling this evening for work - so it might be a day or so until I can give this the attention it deserves - but wanted you to know it's on my radar 👍

[stoiveyp]

Okay I've had a look through this now, and your PR is great work. I think there's actually 3 separate models you're trying to support here with Pins, Stars and Reactions.

1. Pins - they have a model where there is an object with multiple properties and a type - that should be represented as a single PinnedItem object. This way it avoids the overhead of the code sub-typing when it's not necessary. JSON.Net can just handle that as a single object

2. Stars- There is already a StarredItem type that could be moved and used with the appropriate types that could be shared with the events API, and it could be extended over time as you can star ims and other objects (Actually had to use the Node Bolt SDK to find the full list https://github.com/slackapi/bolt-js/blob/86486e01442a80dbd870c24cff4f814a9eed10b0/src/types/events/base-events.ts#L775 )

3. Reactions - Same pattern as Stars, but they'd need ReactionItem as only messages and files are supported.

Hopefully that shows why each is separate enough to warrant a different implementation. But this is great work so far!

[khcnz]

Ok thanks - I'm actually just needing stars right now, but I had done the others as I thought they had the same pattern.

Pins - I'll roll that change back

Re stars/reactions do you propose to move the StarredItem object from WebApi to common Objects folder/namespace? Then we can reuse that and the existing custom deserializer for starreditems?

Re: Other star item types, yes, just didn't want to go too far down the track without feedback :)

[khcnz]

Oh yes I remembered part of the reason I ventured down this track, I thought there were some differences in the data model of the Item between the api and the callback, but seems that actually the current code is wrong

Here is the data from the api

Here is the json from an event callback for stars removed

Here is the current code of an Item

I was confused as the current Item object in the code has a ts element, but actually neither the get stars response or the stars callback item has that field, it should be the date_create field, so can simplify the approach if we correct the Item object?

Am I missing anything about the items object?

[khcnz]

I've reworked it as per the above, and the changes are much simpler/make more sense.

[stoiveyp]

In terms of the common Item object - I don't think you're missing anything, I think it's just that the documentation has improved in the last couple of years. When I originally wrote this library there was a lot missing and I had to make some assumptions. You're helping fix one of them 😁

[stoiveyp]

Hi @khcnz - thank you so much for making this available.

Wanted to keep you updated that I'll be merging this in sometime in the next 8-12hrs; just hoping to release it once I've merged the other changes we've highlighted in this conversation (which I'm sorting out in the background) so there's a single version bump.

[stoiveyp]

This branch has just been published as part of v6.0.0/v6.1.0-beta1 on NuGet After the pins and reactions code were also merged in I had to make minor filename change as we had 3 MessageItems and 3 FileItems (namespaced but confusing) but all up and running now

Thanks again!