Skip to content
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.

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

Cannot add a adaptivecard.Card to action.ShowCard #268

Closed
ArcticXWolf opened this issue Jul 24, 2024 · 5 comments · Fixed by #274
Closed

Cannot add a adaptivecard.Card to action.ShowCard #268

ArcticXWolf opened this issue Jul 24, 2024 · 5 comments · Fixed by #274
Assignees
Labels
bug Something isn't working card format/adaptivecard Adaptive Card support validation
Milestone

Comments

@ArcticXWolf
Copy link

ArcticXWolf commented Jul 24, 2024

Hello,

I think I stumbled upon a bug in the current validation code of the Action.ShowCard. If I try to add this action, I get the following error:

error: specifying a Card is unsupported for Action type "Action.ShowCard": invalid field value

But the field Card is exclusively FOR the action type Action.ShowCard.

In line 1930 of adaptivecard.go the code checks if the action is of type Action.ShowCard. However afterwards in line 1932 it returns an error no matter what.

https://github.com/atc0005/go-teams-notify/blob/master/adaptivecard/adaptivecard.go#L1932

I think we need to catch the return value of line 1930 and then skip line 1932 if it is true. I think in the current implementation you cannot use Action.ShowCard at all.

If I just comment out line 1932, I can use the Action correctly.

@ArcticXWolf
Copy link
Author

ArcticXWolf commented Jul 24, 2024

Oh and I am using the v2.11.0-alpha.1 version and go against the new MS Teams Workflow webhook. It works great so far. #262

@atc0005 atc0005 self-assigned this Jul 24, 2024
@atc0005 atc0005 added bug Something isn't working card format/adaptivecard Adaptive Card support validation labels Jul 24, 2024
@atc0005 atc0005 added this to the v2.11.0 milestone Jul 24, 2024
@atc0005
Copy link
Owner

atc0005 commented Jul 24, 2024

@ArcticXWolf: In line 1930 of adaptivecard.go the code checks if the action is of type Action.ShowCard. However afterwards in line 1932 it returns an error no matter what.

I believe you're right.

I went back over the earlier changes to the validation logic for Action.Validate() and I see what I intended, but missed. I don't believe this is needed any longer:

return fmt.Errorf(
"error: specifying a Card is unsupported for Action type %q: %w",
a.Type,
ErrInvalidFieldValue,
)

I think we need to catch the return value of line 1930 and then skip line 1932 if it is true.

The validation helper collects any potential error as it is used and then returns it or nil here:

// Return the last recorded validation error, or nil if no validation
// errors occurred.
return v.Err()

Incoming PR to address this.

atc0005 added a commit that referenced this issue Jul 24, 2024
The now removed error handling was used prior to refactoring
applied in commit 7ef6d52.

credit: Thanks to @ArcticXWolf for reporting this.

- refs GH-268
- refs GH-251
@atc0005
Copy link
Owner

atc0005 commented Jul 24, 2024

@ArcticXWolf Please test the change in v2.11.0-alpha.2:

Based on your earlier testing I expect that you'll get the same results, but would appreciate the confirmation.

Thanks!

@ArcticXWolf
Copy link
Author

Yes, it works :) Thank you!

@atc0005
Copy link
Owner

atc0005 commented Jul 24, 2024

@ArcticXWolf Thanks for confirming!

Reopening this to serve as a tracking item for the next stable release.

Thanks also for reporting this.

@atc0005 atc0005 reopened this Jul 24, 2024
atc0005 added a commit that referenced this issue Jul 24, 2024
The now removed error handling was used prior to refactoring
applied in commit 7ef6d52.

credit: Thanks to @ArcticXWolf for reporting this.

- refs GH-268
- refs GH-251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working card format/adaptivecard Adaptive Card support validation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants