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

Line handlers #114

Merged
merged 5 commits into from
Feb 16, 2018
Merged

Line handlers #114

merged 5 commits into from
Feb 16, 2018

Conversation

norkans7
Copy link
Contributor

@norkans7 norkans7 commented Feb 15, 2018

Fixes #96


}

return []courier.Event{msgs[0]}, courier.WriteMsgSuccess(ctx, w, r, msgs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be returning all msgs, not just [0]

// },
// {
// "replyToken": "nHuyWiB7yP5Zw52FIkcQobQuGDXCTA",
// "type": "follow",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we not support follows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason why not?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess if we don't support it in the past that's ok to leave it off for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right

if len(lineRequest.Events) > 0 {
return nil, courier.WriteAndLogRequestIgnored(ctx, w, r, channel, "ignoring request, no message")
}
return nil, courier.WriteAndLogRequestError(ctx, w, r, channel, fmt.Errorf("missing message, source or type in the event"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure I understand this case.. is this really an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the events list was empty we return an error, otherwise the event were just ignored by us

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems if the list is empty we should just ignore it too, if its valid JSON that's just fine.

if err != nil {
return status, err
}
status.SetStatus(courier.MsgWired)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any external id to parse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the docs says they just return a 200 and and empty JSON

)

var sendURL = "https://api.line.me/v2/bot/message/push"
var maxMsgLength = 1600
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know what this is exactly from their API docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see it is 2000 on the docs, I update that

@nicpottier nicpottier merged commit e76e9d6 into master Feb 16, 2018
@nicpottier nicpottier deleted the line-handlers branch February 16, 2018 18:53
@nicpottier nicpottier removed the review label Feb 16, 2018
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