Skip to content

Conversation

@yoshinobc
Copy link
Contributor

Motivation

Resolve a task #345
This is to add UnmarshalJSON functionality to TemplateMessage.

Description of the changes

  • template_unmarshal.go file and created the UnmarshalTemplateJSON method. The logic of this method is almost identical to UnmarshalFlexMessageJSON method.
  • Add test code to UnmarshalFlexMessageJSON (template_test.go)

What we considered but did not do

While implementing UnmarshalTemplateJSON, I noticed that UnmarshalJSON is not implemented for CarouselColumn and ImageCarouselColumn. However, considering the possibility that it might have a bad influence on other codes, we added rawColumn and rawImageColumn here and implemented UnmarshalJSON. If there is no risk of having a bad influence on other code, modify CarouselColumn and ImageCarouselColumn to add UnmarshalJSON.

What to do next

There was not enough testing for MarshalJSON in Template, so we will add it.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 82.14% and project coverage change: +0.15 🎉

Comparison is base (40ea0bd) 78.77% compared to head (89dfed9) 78.93%.

❗ Current head 89dfed9 differs from pull request most recent head 5035654. Consider uploading reports for the commit 5035654 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #346      +/-   ##
==========================================
+ Coverage   78.77%   78.93%   +0.15%     
==========================================
  Files          39       40       +1     
  Lines        3600     3712     +112     
==========================================
+ Hits         2836     2930      +94     
- Misses        581      590       +9     
- Partials      183      192       +9     
Impacted Files Coverage Δ
linebot/template_unmarshal.go 82.14% <82.14%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@kkdai kkdai left a comment

Choose a reason for hiding this comment

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

LGTM

@kkdai kkdai merged commit a24603d into line:master May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants