Skip to content

feat: update the manifest model to include the setting for work objects #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vegeris merged 3 commits into from
Oct 21, 2025
Merged

feat: update the manifest model to include the setting for work objects #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af994ce
feat: update the manifest model to include the setting for work objects
vegeris Oct 21, 2025
e9d48b7
update
vegeris Oct 21, 2025
c8987e5
Revert "update"
vegeris Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions internal/shared/types/app_manifest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ type AppFeatures struct {
ManifestShortcutsItems []ManifestShortcutsItem `json:"shortcuts,omitempty" yaml:"shortcuts,flow,omitempty"`
ManifestSlashCommandsItems []ManifestSlashCommandsItem `json:"slash_commands,omitempty" yaml:"slash_commands,flow,omitempty"`
Search *Search `json:"search,omitempty" yaml:"search,flow,omitempty"`
RichPreviews *RichPreviews `json:"rich_previews,omitempty" yaml:"rich_previews,flow,omitempty"`
}

type RichPreviews struct {
EntityTypes []string `json:"entity_types,omitempty" yaml:"entity_types,flow,omitempty"`
Copy link
Member

@zimeg zimeg Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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!

Copy link
Member

@mwbrooks mwbrooks Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

IsActive bool `json:"is_active,omitempty" yaml:"is_active,flow,omitempty"`
Copy link
Contributor Author

@vegeris vegeris Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

}

type AssistantView struct {
Expand Down
11 changes: 11 additions & 0 deletions internal/shared/types/app_manifest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,17 @@ func Test_AppManifest_AppFeatures(t *testing.T) {
},
want: `{"app_home":{},"bot_user":{"display_name":"kubrick"},"search":{"search_function_callback_id":"movie_search","search_filters_function_callback_id":"movie_filters","enable_ai_answers":true}}`,
},
"includes Work Objects settings when provided": {
features: AppFeatures{
BotUser: BotUser{
DisplayName: "business_bot",
},
RichPreviews: &RichPreviews{
EntityTypes: []string{"slack#/entities/file"},
},
Comment on lines +187 to +189
Copy link
Member

@zimeg zimeg Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📠 praise: Thanks for including test, as always!

},
want: `{"app_home":{},"bot_user":{"display_name":"business_bot"},"rich_previews":{"entity_types":["slack#/entities/file"]}}`,
},
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
Expand Down
Loading