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

Add webhook definitions in EventRequestParser #556

Merged
merged 12 commits into from
Dec 25, 2023

Conversation

Yang-33
Copy link
Contributor

@Yang-33 Yang-33 commented Dec 22, 2023

This change allows SDK user to use 6 webhook events defined in https://github.com/line/line-openapi/blob/988429c5737e464bb891b949a874d5f389476681/webhook.yml#L111-L116 by modifying EventRequestParser.

Comment on lines 1681 to 1682
$this->assertInstanceOf(\LINE\Webhook\Model\AttachedModuleContent::class, $module);
$this->assertInstanceOf(\LINE\Webhook\Model\AttachedModuleContent::class, $module, "Expected 'LINE\Webhook\Model\AttachedModuleContent', got '$actualClassName'");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fails even though debug(error) log in parseModuleContent showed its class is AttachedModuleContent. How should I do ...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's resolved by 228f044. Thank you @moririnson san!!!!

@Yang-33 Yang-33 requested a review from moririnson December 22, 2023 06:44
@@ -231,7 +254,7 @@ private static function parseThingsContent($eventData): ThingsContent
{
$thingsContentType = $eventData['things']['type'];
if (!isset(self::$thingsContentType2class[$thingsContentType])) {
return new ThingsContent($eventData['source']);
return new ThingsContent($eventData['things']);
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏

Copy link
Contributor

@moririnson moririnson left a comment

Choose a reason for hiding this comment

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

LGTM 🙏
You're already PHPer.

@Yang-33 Yang-33 closed this Dec 24, 2023
@Yang-33 Yang-33 deleted the add-webhook-for-partners branch December 24, 2023 03:11
@Yang-33 Yang-33 restored the add-webhook-for-partners branch December 25, 2023 00:19
@Yang-33 Yang-33 reopened this Dec 25, 2023
@Yang-33 Yang-33 merged commit 72b6e4d into line:master Dec 25, 2023
@Yang-33 Yang-33 deleted the add-webhook-for-partners branch December 25, 2023 04:17
@eucyt eucyt mentioned this pull request Mar 25, 2025
eucyt added a commit that referenced this pull request Mar 26, 2025
Resolve #673 

## Summary

When a new webhook type is added, we had to update manually
(#664,
#556).
I've created a function to parse the class that uses polymorphism and
fixed it. it prevents human errors in release new Webhook.

## Breaking changes

This PR has some breaking changes to parse Webhook events automatically

### joined.members and left.members will be serialized into the
UserSource classes.

The joined.members and left.members were associative arrays, but now
they are correctly serialized into the UserSource class.

```diff
...
          ["members"]=>
          array(1) {
            [0]=>
-            array(2) {
-              ["type"]=>
-              string(4) "user"
-              ["userId"]=>
-              string(14) "U4af4980629..."
-            }
+            object(LINE\Webhook\Model\UserSource)#256 (2) {
+              ["openAPINullablesSetToNull":protected]=>
+              array(0) {
+              }
+              ["container":protected]=>
+              array(2) {
+                ["type"]=>
+                string(4) "user"
+                ["userId"]=>
+                string(14) "U4af4980629..."
+              }
+            }
...
```

Until now, the elements of members were mistakenly treated as
associative arrays, so we couldn't utilize the methods of UserSource.
With this implementation, the following can now be achieved.

```diff
$parsedEvents = EventRequestParser::parseEventRequest($req->getBody(), $secret, $req->getHeader(HTTPHeader::LINE_SIGNATURE)[0]);
foreach ($parsedEvents->getEvents() as $event) {
  $joinedMembers = $event->getJoined()->getMembers();
  $joinedMemberIds = array_map(function ($member) {
-      return $member["userId"];
+      return $member->getUserId();
  }, $joinedMembers);
}
```

### Default values will be unified to `NULL`.

The default values of array fields were not consistently set to empty
arrays (such as `message.emojis`) or `NULL` (such as `message.
keywords`). Fields with keys that do not exist in the sent webhook event
will have default values unified to `NULL`. For example, if the webhook
does not contain emojis in a text message event, the emoji in
ParsedEvent will change from an empty array to `NULL`.

```diff
...
      ["message"]=>
      object(LINE\Webhook\Model\TextMessageContent)#166 (2) {
        ["openAPINullablesSetToNull":protected]=>
        array(0) {
        }
        ["container":protected]=>
        array(7) {
          ["type"]=>
          string(4) "text"
          ["id"]=>
          string(9) "contentid"
          ["text"]=>
          string(21) "message without emoji"
          ["emojis"]=>
-          array(0) {
-          }
+         NULL
          ["mention"]=>
          NULL
          ["quoteToken"]=>
          NULL
          ["quotedMessageId"]=>
          NULL
        }
      }
...
```
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.

2 participants