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.

Sign up for GitHub

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

Increase character limit for in-app notification messages #2684

Merged
merged 4 commits into from
Sep 3, 2024
Merged

Increase character limit for in-app notification messages #2684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bbaa150
update char limits
hmalik88 Aug 30, 2024
82171e3
Merge branch 'main' into hm/increase-notif-char-limit
hmalik88 Sep 3, 2024
6e724dc
update condition
hmalik88 Sep 3, 2024
721602b
fix test
hmalik88 Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/snaps-rpc-methods/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = deepmerge(baseConfig, {
],
coverageThreshold: {
global: {
branches: 91.81,
branches: 92.07,
functions: 96.96,
lines: 97.51,
statements: 96.97,
Expand Down
20 changes: 16 additions & 4 deletions packages/snaps-rpc-methods/src/restricted/notify.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,22 +229,22 @@ describe('snap_notify', () => {
expect(() =>
getValidatedParams({ type: NotificationType.InApp, message: '' }),
).toThrow(
'Must specify a non-empty string "message" less than 50 characters long.',
'Must specify a non-empty string "message" less than 500 characters long.',
);
});

it('throws an error if the message is not a string', () => {
expect(() =>
getValidatedParams({ type: NotificationType.InApp, message: 123 }),
).toThrow(
'Must specify a non-empty string "message" less than 50 characters long.',
'Must specify a non-empty string "message" less than 500 characters long.',
);
});

it('throws an error if the message is larger than 50 characters', () => {
it('throws an error if the message is larger than or equal to 50 characters for native notifications', () => {
expect(() =>
getValidatedParams({
type: NotificationType.InApp,
type: NotificationType.Native,
message:
'test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg',
}),
Expand All @@ -253,6 +253,18 @@ describe('snap_notify', () => {
);
});

it('throws an error if the message is larger than or equal to 500 characters for in app notifications', () => {
expect(() =>
getValidatedParams({
type: NotificationType.InApp,
message:
'test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg',
}),
).toThrow(
'Must specify a non-empty string "message" less than 500 characters long.',
);
});

it('returns valid parameters', () => {
expect(getValidatedParams(validParams)).toStrictEqual(validParams);
});
Expand Down
16 changes: 15 additions & 1 deletion packages/snaps-rpc-methods/src/restricted/notify.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,27 @@ export function getValidatedParams(params: unknown): NotifyParams {
});
}

const isNotString = !message || typeof message !== 'string';
// Set to the max message length on a Mac notification for now.
if (!message || typeof message !== 'string' || message.length >= 50) {
if (
type === NotificationType.Native &&
(isNotString || message.length >= 50)
) {
throw rpcErrors.invalidParams({
message:
'Must specify a non-empty string "message" less than 50 characters long.',
});
}

if (
type === NotificationType.InApp &&
(isNotString || message.length >= 500)
) {
throw rpcErrors.invalidParams({
message:
'Must specify a non-empty string "message" less than 500 characters long.',
});
}

return params as NotificationArgs;
}
Loading