CGA1123 / slack-ruby-block-kit

A ruby wrapper for Slack's Block Kit
https://rubydoc.info/gems/slack-ruby-block-kit
MIT License
72 stars 30 forks source link

Enforcing BlockKit limits #176

Open bmorton opened 1 year ago

bmorton commented 1 year ago

In looking to replace our internal BlockKit implementation with this one, something that came up is that we'd lose the support we built for enforcing Slack's BlockKit limits on text fields and number of elements in various blocks.

Is this support that you'd be interested in accepting a pull request for? Do you have thoughts about how this should be implemented in this library?

CGA1123 commented 1 year ago

Hey, this would definitely be a welcome addition of functionality to the ecosystem!

I would be interested in hearing about how you approached solving this internally and any strategies in particular for managing these validation failures (and if those have/would impact the design of any interface!).

It might be worth also exploring whether we can leverage existing tools to validate payloads -- rather than having to implement some kind of validation interface into this lib.

It looks like (although somewhat undocumented) it is possible to extract a JSON schema for block kit messages from their online block builder:

This could be functionality that lives within this library or independently as a "validator" for block kit JSON -- which may or may not have been generated via this gem 🤔

bmorton commented 1 year ago

We've got a pretty straightforward system right now:

I agree that using the JSON schema would be the right way to do this. I took a swing at using that gist after digging out the non-truncated one that's buried in there. I couldn't get it to validate properly -- it seemed like it was only for a portion of the BlockKit spec or was too old to be useful, but I only took a pretty quick pass on it.

We've also reached out to Slack to see if they can provide this to us, but we haven't heard anything back yet.

If we were to do the former, I think it'd have to be embedded in the library. If we can take the schema-validator approach, I agree that we could have some broader utility if we keep it independent.

CGA1123 commented 1 year ago

It does seem that maybe they are no longer exposing that schema anywhere anymore.

The current block builder UI now makes an RPC call to /api/blocks.format in order to run validation, rather than having any kind of client-side validation happening it seems.

If Slack don't provide a schema, I think it could be beneficial to scrape one together and keep it up to date over time, especially if it is in a language agnostic form.

To some extent, if there were an exposed schema, most of the code currently in this gem could simply be generated, which might be quite nice and guarantee a level of consistency in the API, depending on how nice the generator was!

bougyman commented 1 year ago

Would be willing to help move this forward, is there a working branch/fork available?

lritter commented 12 months ago

Hey there. I sketched out a possible approach here https://github.com/CGA1123/slack-ruby-block-kit/pull/180 and would love to hear if this might help. The PR adds the ability to flexibly check for limit/schema violations (though doesn't use json schema but rather just checks limits advertised in the documentation) as well as sanitize the json produced during serialization. This allows for, say, option labels to be truncated rather than raising an error. You could imagine implementing your own "limiter" to additionally provide tracing or logging in such cases.

This is just a sketch to illustrate a few different scenarios but I'd love feedback on whether this seems useful. Thanks!

CGA1123 commented 11 months ago

Hey @lritter -- thanks for taking the time to look at this and for providing a potential solution. Much appreciated!

I apologise for the delayed reply. I haven't yet been able to make the time to think this through in full, but wanted to ACK this and will do my best to make some time in the evening this week or over this coming weekend.

In the meantime, would be interested to hear from @bougyman + @bmorton about thoughts and whether this would cater to your use-cases -- you are better placed than I am to shed light on clear requirements/wants from a solution!

lritter commented 11 months ago

Appreciate it @CGA1123!

I'll say that one of the things I don't like about my solution is that it mixes the "validation" concern with the "make it work" concern (modifying supplied values such as long labels to allow things to work). Doing the "make it work" stuff at the application layer makes sense but being able to hook into the framework to do this greatly eases development and reduces the chance of missing stuff. The PR here seems like "the simplest thing that could work" but I'm definitively curious about other options.

Thanks!

CGA1123 commented 11 months ago

I think the idea of a serialisation middleware stack is slick, and allows for all sorts of flexibility!

There's then probably a set of common implementations for field types that have common restrictions (text length, enum types, etc...) which probably need to have some different ways to "fail": notify, raise, "try to autofix" (e.g. truncate).

I like this because it also keeps the door open for curating or finding a JSON-Schema based approach for all these components, and potentially leveraging existing tooling for validation around that.