Skip to content

Commit

Permalink
[1] Cleanup unused field, fix and define action payload type (#366)
Browse files Browse the repository at this point in the history
- [refactor: remove issuedAt](8906575)
  - `issuedAt` logic was initially introduced before 2018 (https://github.com/cofacts/rumors-line-bot/pull/113/files) and was long replaced by sessionId
  - This PR removes the remaining `issuedAt`
- [fix(chatbotState): replace `PostbackEvent` with actual type used in handlers](d2cf992) 
  - The new type describes the weird payload event (`type: payload` + `input`) that the chatbot uses
  - Many`as ChatbotEvent` can be removed as the current `ChatbotEvent` can precisely describe the event being passed
- [refactor(webhook): Read / write of postback data is protected by type](f23f668)
  - Define the payload action that is currently being used
- [refactor(gql): narrow the type of GraphQL response](9bc912b)
  - Better `data` & `error` that matches the actual scenario
  • Loading branch information
MrOrz authored Oct 18, 2023
2 parents 197a018 + df10519 commit 33a8bbc
Show file tree
Hide file tree
Showing 15 changed files with 91 additions and 77 deletions.
40 changes: 20 additions & 20 deletions src/lib/gql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ type QV = {
variables?: object;
};

type Resp = { data: object | null; errors: { message: string }[] | undefined };

// Maps URL to dataloader. Cleared after batched request is fired.
// Exported just for unit test.
export const loaders: Record<string, Dataloader<QV, object>> = {};
export const loaders: Record<string, Dataloader<QV, Resp>> = {};

/**
* Returns a dataloader instance that can send query & variable to the GraphQL endpoint specified by `url`.
Expand All @@ -26,26 +28,24 @@ export const loaders: Record<string, Dataloader<QV, object>> = {};
function getGraphQLRespLoader(url: string) {
if (loaders[url]) return loaders[url];

return (loaders[url] = new Dataloader<QV, object>(
async (queryAndVariables) => {
// Clear dataloader so that next batch will get a fresh dataloader
delete loaders[url];
return (loaders[url] = new Dataloader<QV, Resp>(async (queryAndVariables) => {
// Clear dataloader so that next batch will get a fresh dataloader
delete loaders[url];

// Implements Apollo's transport layer batching
// https://www.apollographql.com/blog/apollo-client/performance/query-batching/#1bce
//
return (
await fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'x-app-secret': process.env.APP_SECRET ?? '',
},
body: JSON.stringify(queryAndVariables),
})
).json();
}
));
// Implements Apollo's transport layer batching
// https://www.apollographql.com/blog/apollo-client/performance/query-batching/#1bce
//
return (
await fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'x-app-secret': process.env.APP_SECRET ?? '',
},
body: JSON.stringify(queryAndVariables),
})
).json();
}));
}

// Usage:
Expand Down
25 changes: 23 additions & 2 deletions src/types/chatbotState.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Message, MessageEvent, WebhookEvent } from '@line/bot-sdk';
import type { Message, MessageEvent } from '@line/bot-sdk';

export type ChatbotState =
| '__INIT__'
Expand Down Expand Up @@ -28,7 +28,17 @@ type ArgumentedEventParams = {
input: string;
};

export type ChatbotEvent = (WebhookEvent | ServerChooseEvent) &
export type ChatbotEvent = (
| MessageEvent
| ServerChooseEvent
/**
* A special format of postback that Chatbot actually uses: postback + input (provided in `ArgumentedEventParams`)
* @FIXME Replace with original PostbackEvent and parse its action to support passing more thing than a string
*/
| {
type: 'postback';
}
) &
ArgumentedEventParams;

export type Context = {
Expand Down Expand Up @@ -67,3 +77,14 @@ export type ChatbotStateHandlerReturnType = Omit<
export type ChatbotStateHandler = (
params: ChatbotStateHandlerParams
) => Promise<ChatbotStateHandlerReturnType>;

/**
* The data that postback action stores as JSON.
*
* @FIXME Replace input: string with something that is more structured
*/
export type PostbackActionData = {
input: string;
sessionId: number;
state: ChatbotState;
};
2 changes: 1 addition & 1 deletion src/webhook/handleInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default async function handleInput(
event = {
type: 'postback',
input: articleId,
} as ChatbotEvent;
};
return await handlePostback({ data }, 'CHOOSING_ARTICLE', event, userId);
} else if (event.input === TUTORIAL_STEPS['RICH_MENU']) {
state = 'TUTORIAL';
Expand Down
8 changes: 4 additions & 4 deletions src/webhook/handlePostback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import { Message } from '@line/bot-sdk';
/**
* Given input event and context, outputs the new context and the reply to emit.
*
* @param {Object<data>} context The current context of the bot
* @param {*} state The input state
* @param {*} event The input event
* @param {*} userId LINE user ID that does the input
* @param context The current context of the bot
* @param state The input state
* @param event The input event
* @param userId LINE user ID that does the input
*/
export default async function handlePostback(
{ data = {} },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ Object {
"input": "AV--O3nfyCdS-nWhujMD",
"type": "server_choose",
},
"issuedAt": undefined,
"replies": Array [
Object {
"text": "💡 Someone on the internet replies to the message:",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ Object {
"timestamp": 1518964687709,
"type": "postback",
},
"issuedAt": 1518964688672,
"replies": Array [
Object {
"text": "💡 Someone on the internet replies to the message:",
Expand Down Expand Up @@ -400,7 +399,6 @@ Object {
"timestamp": 1518964687709,
"type": "postback",
},
"issuedAt": 1518964688672,
"replies": Array [
Object {
"text": "💡 Someone on the internet replies to the message:",
Expand Down
10 changes: 0 additions & 10 deletions src/webhook/handlers/__tests__/choosingArticle.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ it('should select article by articleId', async () => {
data: '{"input":"article-id","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}',
},
},
issuedAt: 1505314295017,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
};

Expand Down Expand Up @@ -83,7 +82,6 @@ it('throws ManipulationError when articleId is not valid', async () => {
type: 'postback',
input: 'article-id',
},
issuedAt: 1505314295017,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
};

Expand All @@ -109,7 +107,6 @@ it('should select article and have OPINIONATED and NOT_ARTICLE replies', async (
data: '{"input":"article-id","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}',
},
},
issuedAt: 1511633232970,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
replies: undefined,
};
Expand Down Expand Up @@ -180,7 +177,6 @@ it('should select article with no replies', async () => {
data: '{"input":"article-id","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}',
},
},
issuedAt: 1511702208730,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
replies: undefined,
};
Expand Down Expand Up @@ -240,7 +236,6 @@ it('should select article and choose the only one reply for user', async () => {
data: '{"input":"article-id","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}',
},
},
issuedAt: 1511702208730,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
replies: undefined,
};
Expand Down Expand Up @@ -288,7 +283,6 @@ it('should block non-postback interactions', async () => {
timestamp: 1511702208226,
message: { type: 'text', id: '7049700770815', text: '10' },
},
issuedAt: 1511702208730,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
replies: undefined,
};
Expand All @@ -314,7 +308,6 @@ it('should select article and slice replies when over 10', async () => {
data: '{"input":"article-id","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}',
},
},
issuedAt: 1511633232970,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
replies: undefined,
};
Expand All @@ -337,7 +330,6 @@ it('should ask users if they want to submit article when user say not found', as
data: `{"input":"${POSTBACK_NO_ARTICLE_FOUND}","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}`,
},
},
issuedAt: 1511633232970,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
replies: undefined,
};
Expand Down Expand Up @@ -375,7 +367,6 @@ it('should ask users if they want to submit image article when user say not foun
data: `{"input":"${POSTBACK_NO_ARTICLE_FOUND}","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}`,
},
},
issuedAt: 1511633232970,
userId: 'Uc76d8ae9ccd1ada4f06c4e1515d46466',
replies: undefined,
};
Expand Down Expand Up @@ -413,7 +404,6 @@ it('should create a UserArticleLink when selecting a article', async () => {
data: '{"input":"article-id","sessionId":1497994017447,"state":"CHOOSING_ARTICLE"}',
},
},
issuedAt: 1505314295017,
userId,
};

Expand Down
3 changes: 0 additions & 3 deletions src/webhook/handlers/__tests__/choosingReply.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ describe('should select reply by replyId', () => {
data: '{"input":"AWDZeeV0yCdS-nWhuml8","state":"CHOOSING_REPLY"}',
},
},
issuedAt: 1518964688672,
userId: 'Uaddc74df8a3a176b901d9d648b0fc4fe',
replies: [],
};
Expand Down Expand Up @@ -106,7 +105,6 @@ it('should block non-postback interactions', async () => {
input: '123',
timestamp: 1518964687709,
},
issuedAt: 1518964688672,
userId: 'Uaddc74df8a3a176b901d9d648b0fc4fe',
replies: [],
};
Expand All @@ -132,7 +130,6 @@ it('should handle graphql error gracefully', async () => {
data: '{"input":"AWDZeeV0yCdS-nWhuml8","state":"CHOOSING_REPLY"}',
},
},
issuedAt: 1518964688672,
userId: 'Uaddc74df8a3a176b901d9d648b0fc4fe',
replies: [],
};
Expand Down
1 change: 0 additions & 1 deletion src/webhook/handlers/__tests__/tutorial.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const param = {
input: undefined,
timestamp: 1519019734813,
},
issuedAt: 1519019735467,
userId: 'Uaddc74df8a3a176b901d9d648b0fc4fe',
replies: undefined,
};
Expand Down
4 changes: 2 additions & 2 deletions src/webhook/handlers/choosingReply.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ function createShareBubble(articleId, fullArticleText, replyTypeEnumValue) {
}

export default async function choosingReply(params) {
let { data, state, event, issuedAt, userId, replies } = params;
let { data, state, event, userId, replies } = params;

if (event.type !== 'postback' && event.type !== 'server_choose') {
throw new ManipulationError(t`Please choose from provided options.`);
Expand Down Expand Up @@ -182,5 +182,5 @@ export default async function choosingReply(params) {
visitor.event({ ec: 'Reply', ea: 'Type', el: GetReply.type, ni: true });
visitor.send();

return { data, event, issuedAt, userId, replies };
return { data, event, userId, replies };
}
4 changes: 2 additions & 2 deletions src/webhook/handlers/defaultState.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
export default function defaultState(params) {
let { data, event, issuedAt, userId, replies } = params;
let { data, event, userId, replies } = params;

replies = [
{
type: 'text',
text: '我們看不懂 QQ\n大俠請重新來過。',
},
];
return { data, event, issuedAt, userId, replies };
return { data, event, userId, replies };
}
4 changes: 2 additions & 2 deletions src/webhook/handlers/processMedia.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {
FlexMessage,
FlexComponent,
} from '@line/bot-sdk';
import { Context } from 'src/types/chatbotState';
import { ChatbotStateHandlerParams } from 'src/types/chatbotState';

import {
getLineContentProxyURL,
Expand All @@ -28,7 +28,7 @@ const CIRCLED_DIGITS = '⓪①②③④⑤⑥⑦⑧⑨⑩⑪';
const SIMILARITY_THRESHOLD = 0.95;

export default async function (
{ data = {} as Context },
{ data }: { data: ChatbotStateHandlerParams['data'] },
event: MessageEvent,
userId: string
) {
Expand Down
11 changes: 6 additions & 5 deletions src/webhook/handlers/tutorial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '@line/bot-sdk';
import { CreateReplyMessagesReplyFragment } from 'typegen/graphql';
import {
ChatbotState,
ChatbotStateHandlerParams,
ChatbotStateHandlerReturnType,
Context,
Expand Down Expand Up @@ -84,15 +85,15 @@ function createImageTextBubble(imageUrl: string, text: string): FlexBubble {
}

/**
* @param {string} label Act as quickReply's label and postback's input and displayText
* @param {number} sessionId Search session ID
* @param {string} postbackState Used by `handleInput` to determine which handler to call
* @returns {object} quickReply items object
* @param label Act as quickReply's label and postback's input and displayText
* @param sessionId Search session ID
* @param postbackState Used by `handleInput` to determine which handler to call
* @returns quickReply items object
*/
function createQuickReplyPostbackItem(
label: string,
sessionId: number,
postbackState: string
postbackState: ChatbotState
): QuickReplyItem {
return {
type: 'action',
Expand Down
16 changes: 10 additions & 6 deletions src/webhook/handlers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {
} from 'typegen/graphql';
import { getArticleURL, createTypeWords } from 'src/lib/sharedUtils';
import { sign } from 'src/lib/jwt';
import { ChatbotState, PostbackActionData } from 'src/types/chatbotState';

const splitter = new GraphemeSplitter();

Expand All @@ -36,17 +37,20 @@ export function createPostbackAction(
input: string,
displayText: string,
sessionId: number,
state: string
state: ChatbotState
): Action {
// Ensure the data type before stringification
const data: PostbackActionData = {
input,
sessionId,
state,
};

return {
type: 'postback',
label,
displayText,
data: JSON.stringify({
input,
sessionId,
state,
}),
data: JSON.stringify(data),
};
}

Expand Down
Loading

0 comments on commit 33a8bbc

Please sign in to comment.