IDEMSInternational / open-app-builder

PLH App Frontend
GNU General Public License v3.0
5 stars 24 forks source link

Feat: user import action #2270

Closed chrismclarke closed 3 months ago

chrismclarke commented 3 months ago

PR Checklist

Description

Main Changes

Additional Changes

Review Notes

Testing is a bit tricky as it requires a development server running locally as well as the code. Ideally everything would run from deployment preview using the debug repo, however it looks like the action is not currently working as intended (to review). For testing locally:

Assuming postgres db already running locally then just api can be run via:

yarn workspace api start;
yarn start;

Or if running as docker container will need to update the endpoint in src\app\shared\services\server\interceptors.ts to be http://localhost/api and run

yarn workspace api build;
yarn workspace server start;

I also had a few issues with a previous docker implementation running locally (and conflicts with postgres running at same port), and ended up having to wipe local docker containers using

yarn workspace server dangerously_wipe

It also requires the debug_user_actions sheet, either running locally or synced to debug deployment.

See example in video below

Author Notes

Fields starting with _ are omitted, such as _app_skin, _app_theme,_app_version. Whilst some of these might be useful to populate (e.g. importing also sets app skin), the default behaviour is to ignore to avoid confusion with services that set these fields programmatically. It may be useful to review protected field handling in the future to make clear what can/can't be overwritten.

I've added a debug_user_actions sheet, not sure whether this should be considered a debug or example sheet, feel free to update or adapt as useful

It's not totally clear why there was a 30 character limit on inputs (implemented 2 years ago by external consultants), the web spec itself limits at 2^19 (524288) for 512kb data equivalence, however the code now uses a fallback of -1 which should have the same effect. If we do really want a hardcoded limit suggest at least 36 characters to avoid having to hardcode override whenever using a user id input

Dev Notes

Future TODOs

Git Issues

Closes #2245

Screenshots/Videos

Screenity video - Apr 2, 2024.webm

chrismclarke commented 3 months ago

The core code for the action looks good. Already having the endpoint makes it reasonably straightforward – are there any implications of what Ian and Chris M were proposing about making the API server more secure (I can't find the mattermost thread atm)?

I've added https://github.com/IDEMSInternational/parenting-app-ui/issues/2277, could be good to discuss on future call @esmeetewinkel - could we add to next agenda?