openstad / openstad-api Goto Github PK

The mail routine needs a site config to determine which transporter to use. Since it seems highly unlikely that different mail transports would be used for different sites this should be changed. One mail transporter for the whole application should suffice.

When this is done it would be possible to merge messages to the same email address into one email, even when hey target different sites, in e.g. src/cron/send_site_issues_notifications.js

It would be nice to move email templates from the site config to a new field.

site.config is a json field with a lot of data. All this data is requested from the database again and again.

The email templates are currently stored in site.config. This means that these templates are also requested from the database again and again. But this data is hardly ever used.

Moving these templates to another field in the site table, and only request those fields when required, would significantly reduce ttraffic between the API and database server, and between the API and the frontend server.

Whenever the sanitize function (src/util/sanitize.js) is called with null, it returns the string 'null'.

When does this matter?
When adding an oauth client which does not have lastName as a required field, the lastName will be set tonull upon creating an account and going through the required fields page. After logging in, the API creates a user from these values. The User model has lastName set to allow null values, but the setter is always passing the value through to the sanitize util. This means that when you create a user called 'Admin' in the oAuth environment, the user will be called 'Admin null' in the API environment. This can be seen in the resource-form-widget's form header for example.

The code contains two notification services.

The first can be found in ./src/services/notificationService.js which is used by ./src/services/eventService.js. This uses rulesets, and seems to be an attempt towards a generic and flexible notification system. The problem is that it is only used by Submissions.

The second can be found in ./src/notifications.js. It uses queues which are processed by crons. This one is used by the rest of the API.

These services should be merged.

Since PR #254 a locking mechanism is available.

The next step should be to create a generic use of this mechanism by cron tabs.

module.exports = {
	cronTime: '0 2 */6 * * *',
	runOnInit: false,
	onTick: createLockForCron(async function() {
	  // cron logic
	})
};