Skip to content

Conversation

@AramAlsabti
Copy link
Contributor

This PR adds basic validation to IoT device metadata when submitted. It also fixed a TS error which came of one of the dependabots updating the package.lock file.
Supposedly, the mqtt package is now missing a dependency which this PR adds back. Related PR

Related frontend PR

Copy link
Contributor

@GufCab GufCab left a comment

Choose a reason for hiding this comment

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

Minor changes, can be merged after fixing


for (const key of Object.keys(json)) {
if (typeof key !== "string" || key.trim() === "") {
this.message = `The key whose value is "${json[key]}" must be a valid text value`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use proper translation key (+ translation in frontend)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I've done away with hinting at which row has the error. The current setup (i.e. throwing a generic BadRequestException in nest.js) doesn't easily allow additional info to be passed to the frontend. That, or I'm blind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case you have a suggestion, I'll leave this open. Otherwise, you can merge this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Translation key is fine.

@AramAlsabti AramAlsabti requested a review from GufCab March 16, 2022 16:33
@GufCab GufCab merged commit cf5f089 into stage Mar 16, 2022
@AramAlsabti AramAlsabti deleted the feature_759-iot-device-metadata branch June 23, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants