Added Blocks support, as well as adjusted for new SearchSort enums returned by the Slack LoginResponse model#178
Conversation
|
There's already support about to be added for Blocks by @clockworkcoding @ #172 |
| @@ -0,0 +1,501 @@ | |||
| namespace SlackAPI | |||
There was a problem hiding this comment.
This whole file has wrong indentation (should be 4 spaces)
There was a problem hiding this comment.
While I do like the more polymorphic approach to the Block support, it seems quite a bit more complex.
@clockworkcoding do you think this is worthwhile to merge in instead of your simpler implementation?
My biggest complaint against this one is that it seems considerably more complex
There was a problem hiding this comment.
It definitely is more complex - the main reason I developed it with this is that I wanted to be able to dynamically generate the message without having to memorize what fields each block or element type has. This was actually originally an addition to the slack-api-csharp library that implements Events, as I built a SlackBot that uses JSON templates to dynamically build block messages and dialogs with data from a SQL server upon user request. (Hence the necessity for polymorphism, so I don't have to check a billion things whenever I try to parse the template.)
And I'm using this library in a VB.NET winforms application at my job (I didn't choose the language, the application is already very expansive and we plan to move it to a web-based platform), so whenever I get a chance to work with ACTUAL object-oriented stuff, I really go for it.
There was a problem hiding this comment.
So, yeah - very niche use case, and if new block and/or element types are added, this code would need to be adjusted to account for that. There is probably a simpler way to do it that would enable dynamic generation of block-based messages, but I very much wanted to keep them distinct (also, SlackTexts can be used in Field for Section Blocks, which also takes Image, so that added another layer of complexity.)
There was a problem hiding this comment.
I went with a simple approach for two reasons:
- To keep consistent with the rest of the api
- To allow consumers to use new element types that reuse the same fields without waiting for this api to update.
I like the ease of use this approach provides, but the lack of consistency might be an issue, until or unless we want to provide a similar interface throughout.
There was a problem hiding this comment.
Personally I like this approach, but yeah it breaks apart from the convention of the rest of the library.
Again, personally, it's been on my mind for a while that the whole library needs to be overhauled to follow a simpler approach (re: ease-of-use) to hopefully prevent some of the issues I've been seeing of people having trouble with using the library in general. To be quite honest, I wrote this whole library initially when I was still very much a novice at C# and added in too many features that most people probably don't know about / I don't even remember at this point. I wonder if there's a nice medium we could find that allows adding new features (without waiting for library updates) and is easier to use.
@gpailler , sorry to drag you in, but I'd like your opinion on this.
3 participants
Added support for Blocks to all API methods, and also accounted for two new enum values (not_set, relevant) for SearchSort from the Slack LoginResponse -> Self -> Preferences model. (Relevant seems to be solely used for search_channel_sort and search_people_sort [two new SearchSort values in the response that I also accounted for], but it's possible it could be set to that for the overall search_sort as well.)