feat: update the manifest model to include the setting for work objects#247
Conversation
e5feb56 to
de46401
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #247 +/- ##
==========================================
- Coverage 63.02% 62.99% -0.04%
==========================================
Files 212 212
Lines 21857 21857
==========================================
- Hits 13776 13769 -7
- Misses 7015 7021 +6
- Partials 1066 1067 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
de46401 to
af994ce
Compare
zimeg
left a comment
There was a problem hiding this comment.
@vegeris LGTM! I find these values appear in app settings after installing or updating a manifest 🤩
No comments requesting change but I did add a changelog section to this PR for upcoming release notes - please of course make edits to what makes more sense! 📚
Otherwise, let's get this merged and perhaps released soon?
| } | ||
|
|
||
| type RichPreviews struct { | ||
| EntityTypes []string `json:"entity_types,omitempty" yaml:"entity_types,flow,omitempty"` |
There was a problem hiding this comment.
📝 though: The omitempty attribute worries me with slices since it's not obvious how to keep an empty list of entity types, but I notice the API errors in that case so no worries!
There was a problem hiding this comment.
Yea, this is the common struggle of matching the API defaults with Golang language rules. Seems like we'll want to keep the omitempty to align with the API.
| RichPreviews: &RichPreviews{ | ||
| EntityTypes: []string{"slack#/entities/file"}, | ||
| }, |
There was a problem hiding this comment.
📠 praise: Thanks for including test, as always!
This reverts commit e9d48b7.
|
|
||
| type RichPreviews struct { | ||
| EntityTypes []string `json:"entity_types,omitempty" yaml:"entity_types,flow,omitempty"` | ||
| IsActive bool `json:"is_active,omitempty" yaml:"is_active,flow,omitempty"` |
There was a problem hiding this comment.
I was on the fence about including this property but after some discussion, we might want to support the dev setting this rather than needing to remove the entire rich_previews setting to disable (so I added it back)
mwbrooks
left a comment
There was a problem hiding this comment.
✅ Looks good to me, thanks for adding support for work objects!
📝 I believe this would be a semver:minor because we're adding a new feature (work object support).
Changelog
The app manifest values for
entity_typesare now gathered when reading an app manifest.Summary
This PR updates the app manifest model to include work object settings.
Test
Requirements