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

New article creation flow #172

Merged
merged 15 commits into from May 9, 2020
Merged

New article creation flow #172

merged 15 commits into from May 9, 2020

Conversation

MrOrz
Copy link
Member

@MrOrz MrOrz commented Apr 12, 2020

This PR rewrites initState handler, and creates askingArticleSubmissionConsent handler that replaces the original askingArticleSource handler.

What's covered

Chatbot Interactions within the red circle (LIFF not included)
state

Other things include:

  • To support Postback buttons should be usable for the entire search session #49 , we remove issueAtbecause it changes every reply. data.sessionId is introduced instead. It changes only when the user submits new message.
  • We are replacing all template reply with flex reply. This brings us benefits like:
    • Flex message is supported in desktop, while template message don't.
    • We no longer need to ask users to type "1", "2", "3" on desktop. This also removes the need to check user input lengh <= 3. Any text input that is not from LIFF can be considered as the start of a new search session.
  • Use JWT as nonce in LIFF URLs. Since the JWT includes exp (expire timestamp claim), MockDate is introduced so that JWT can be deterministic across test runs.
  • Adds mock to ga and asserts what is being sent to Google analytics in unit tests.
  • Moves some of constants / functions that is going to share with LIFF to src/lib/sharedUtils.
  • Adds ManipulationError. It differs from normal runtime error, because handleInput will catch ManipulationError and display its message in a nice flex reply. Very useful when we need to halt processing user input and display instruction to the user in reply.
  • Removes isNonsenseText() logic because it's no longer used.
  • Fix article search result (flex carousel) unit test in initState.

What's not covered

  1. CHOOSING_ARTICLE and other state handlers are not updated accordingly, thus all other flows are not working yet
  2. LIFF implementation is not done yet. The flow is tested by manually inputting prefix + option on LINE, mimicking user interaction in LIFF.
  3. JWT token verification is not implemented on API @auth directive yet. Will implement when implementing LIFF.

Screenshot

Submission flow with valid source

image
image

Submission flow with source that cannot proceed

image
image

Image text not found

@MrOrz MrOrz self-assigned this Apr 12, 2020
@MrOrz MrOrz added this to In progress in [Cofacts Next] 告知 Chatbot 使用者新回應 via automation Apr 12, 2020
@coveralls
Copy link

Pull Request Test Coverage Report for Build 699

  • 56 of 57 (98.25%) changed or added relevant lines in 6 files are covered.
  • 60 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-19.2%) to 77.488%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/webhook/handlers/choosingArticle.js 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
src/webhook/handlers/choosingArticle.js 60 0%
Totals Coverage Status
Change from base Build 690: -19.2%
Covered Lines: 231
Relevant Lines: 295

💛 - Coveralls

@MrOrz MrOrz marked this pull request as ready for review April 12, 2020 08:12
@MrOrz MrOrz changed the base branch from svelte-liff to dev May 1, 2020 07:38

// Track text message type send by user
const visitor = ga(userId, state, event.input);
visitor.event({ ec: 'UserInput', ea: 'MessageType', el: 'text' });
visitor.event({ ec: 'UserInput', ea: 'MessageType', el: event.message.type });
Copy link
Member

Choose a reason for hiding this comment

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

If process.env.IMAGE_MESSAGE_ENABLED is not true, image messages will not be tracked.
Maybe we can extract this along with index.js L168, index.js L179 to the beginning of process message.

[Cofacts Next] 告知 Chatbot 使用者新回應 automation moved this from In progress to Reviewer approved May 8, 2020
@MrOrz MrOrz merged commit 122be66 into dev May 9, 2020
[Cofacts Next] 告知 Chatbot 使用者新回應 automation moved this from Reviewer approved to Done May 9, 2020
@MrOrz MrOrz deleted the new-flow branch June 16, 2020 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants