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

262 show template errors #271

Merged
merged 19 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
25 changes: 0 additions & 25 deletions frontend/packages/host/dist/index.html

This file was deleted.

41 changes: 22 additions & 19 deletions frontend/packages/host/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,28 @@ const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');

class DummyWebpackPlugin {
apply(compiler) {
compiler.hooks.run.tap('DummyWebpackPlugin', () => { });
compiler.hooks.run.tap('DummyWebpackPlugin', () => {});
}
}

module.exports = env => {
const isProduction = !!env.production;
const bundleAnalyzerPlugin = !!env.stats
? new BundleAnalyzerPlugin({
/**
* In "server" mode analyzer will start HTTP server to show bundle report.
* In "static" mode single HTML file with bundle report will be generated.
* In "json" mode single JSON file with bundle report will be generated
*/
analyzerMode: 'disabled',
generateStatsFile: true,
})
/**
* In "server" mode analyzer will start HTTP server to show bundle report.
* In "static" mode single HTML file with bundle report will be generated.
* In "json" mode single JSON file with bundle report will be generated
*/
analyzerMode: 'disabled',
generateStatsFile: true,
})
: new DummyWebpackPlugin();

return {
experiments: {
syncWebAssembly: true,
},
entry: './src/index',
mode: isProduction ? 'production' : 'development',
devtool: isProduction ? undefined : 'source-map',
Expand Down Expand Up @@ -76,19 +79,19 @@ module.exports = env => {
exclude: /node_modules/,
options: isProduction
? {
/* ts-loader options */
}
/* ts-loader options */
}
: {
/* swc-loader options */
jsc: {
parser: {
dynamicImport: true,
syntax: 'typescript',
tsx: true,
/* swc-loader options */
jsc: {
parser: {
dynamicImport: true,
syntax: 'typescript',
tsx: true,
},
target: 'es2015',
},
target: 'es2015',
},
},
},
{
test: /\.css$/,
Expand Down
1 change: 1 addition & 0 deletions frontend/packages/system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"dependencies": {
"@notify-frontend/common": "^0.0.1",
"@notify-frontend/config": "^0.0.0",
"@openmsupply/tera-web": "^0.1.0",
"react": "^18.0.0",
"react-dom": "^18.0.0",
"zxcvbn-typescript": "^5.0.1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export const BaseNotificationEditPage = <T extends BaseNotificationConfig>({
sqlRecipientLists={sqlRecipientLists?.nodes ?? []}
/>
</AppBarContentPortal>
<Grid flexDirection="column" display="flex" gap={2}>
<Grid flexDirection="column" display="flex" gap={2} width={'100%'}>
<Box sx={{ paddingLeft: '10px' }}>
<CustomForm draft={draft} onUpdate={onUpdate} />
{errorMessage ? (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useState } from 'react';
import {
BasicTextInput,
Box,
Expand All @@ -12,6 +12,7 @@ import {
import { ScheduledNotification } from '../../types';
import { SqlQuerySelector } from '../../components';
import { useNotificationQueries } from 'packages/system/src/Queries/api';
import { validateTemplate } from './tera';

type ScheduledNotificationEditFormProps = {
onUpdate: (patch: Partial<ScheduledNotification>) => void;
Expand All @@ -38,6 +39,12 @@ export const ScheduledNotificationEditForm = ({
draft,
}: ScheduledNotificationEditFormProps) => {
const t = useTranslation('system');
const [subjectTemplateError, setSubjectTemplateError] = useState<
string | null
>(null);
const [bodyTemplateError, setBodyTemplateError] = useState<string | null>(
null
);

const { queryParams } = useQueryParamsState();
queryParams.first = 1000; // Set a high limit to ensure all queries are fetched, if we end up with over 1000 queries we'll need a new solution, or if this is too slow...
Expand All @@ -46,26 +53,45 @@ export const ScheduledNotificationEditForm = ({
const queries = data?.nodes ?? [];

return (
<Box paddingTop={1}>
<Box paddingTop={1} width={'100%'} paddingRight={'14px'}>
<FormRow title={t('label.details')}>
<BasicTextInput
autoFocus
helperText={subjectTemplateError}
error={!!subjectTemplateError}
value={draft.subjectTemplate}
required
onChange={e =>
onChange={e => {
onUpdate({
subjectTemplate: e.target
.value as ScheduledNotification['subjectTemplate'],
})
}
});
try {
validateTemplate(e.target.value);
setSubjectTemplateError(null);
} catch (e) {
setSubjectTemplateError(e as string);
}
}}
label={t('label.subject-template')}
InputLabelProps={{ shrink: true }}
fullWidth
/>
</FormRow>
<FormRow title="">
<BufferedTextArea
helperText={bodyTemplateError}
error={!!bodyTemplateError}
value={draft.bodyTemplate}
onChange={e => onUpdate({ bodyTemplate: e.target.value })}
onChange={e => {
onUpdate({ bodyTemplate: e.target.value });
try {
validateTemplate(e.target.value);
setBodyTemplateError(null);
} catch (e) {
setBodyTemplateError(e as string);
}
}}
label={t('label.body-template')}
InputProps={{
sx: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
NotificationConfigRowFragment,
useNotificationConfigs,
} from '../../api';
import { validateTemplate } from './tera';

export const ScheduledNotificationEditPage = () => {
const t = useTranslation('system');
Expand Down Expand Up @@ -55,9 +56,19 @@ export const ScheduledNotificationEditPage = () => {
await update({ input: inputs.update });
};

const isValidTemplate = (template: string) => {
try {
validateTemplate(template);
return true;
} catch (e) {
return false;
}
};

const isInvalid =
!draft.title ||
// nothing selected
!isValidTemplate(draft.subjectTemplate) ||
!isValidTemplate(draft.bodyTemplate) ||
Copy link
Collaborator Author

@lache-melvin lache-melvin Nov 27, 2023

Choose a reason for hiding this comment

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

Would we ever send a notification with a subject but no body? Or a blank subject and just a body? i.e. do both subject and body need to exist, as well as being valid, or just that they need to be valid if they do exist?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should always have a body.
Telegram only renders the body, subject is just for email in current design.

// no recipients selected
(!draft.recipientListIds.length &&
!draft.recipientIds.length &&
Expand Down
25 changes: 25 additions & 0 deletions frontend/packages/system/src/Notifications/Pages/Scheduled/tera.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { renderOneOff } from '@openmsupply/tera-web';

const isTemplateError = (err: string) => {
return err.startsWith('Template');
};
const isParamsError = (err: string) => {
return err.startsWith('Parameter');
};

export const validateTemplate = (template: string) => {
try {
renderOneOff(template, JSON.stringify({}));
} catch (e) {
if (typeof e === 'string') {
if (isTemplateError(e)) {
throw e;
}
if (isParamsError(e)) {
// Missing params are not an error when validating the template
return;
}
}
throw 'Unknown error :' + e;
}
};
5 changes: 5 additions & 0 deletions frontend/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2664,6 +2664,11 @@
dependencies:
"@octokit/openapi-types" "^11.2.0"

"@openmsupply/tera-web@^0.1.0":
version "0.1.0"
resolved "https://registry.yarnpkg.com/@openmsupply/tera-web/-/tera-web-0.1.0.tgz#95b71a5380feee8ab260adb7a407f2b0f1d1821e"
integrity sha512-7SwRnVuBiKz6qiISAlPRna69mqPRMOcDMrym0lS6Of33lwBFo+VRwlwjGDfivI57WeFqODDJcfwufnp2Dx/1bQ==

"@parcel/[email protected]":
version "2.2.0"
resolved "https://registry.yarnpkg.com/@parcel/watcher-android-arm64/-/watcher-android-arm64-2.2.0.tgz#3d1a71f251ba829ab884dfe119cc4f4c49c7222b"
Expand Down