Skip to content

Conversation

@jona159
Copy link
Contributor

@jona159 jona159 commented Feb 3, 2026

Type of Change

  • Dependency upgrade
  • Bug fix (non-breaking change)
  • Breaking change
    • e.g. a fixed bug or new feature that may break something else
  • New feature
  • Code quality improvements
    • e.g. refactoring, documentation, tests, tooling, ...

Implementation

Checklist

  • I gave this pull request a meaningful title
  • My pull request is targeting the dev branch
  • I have added documentation to my code
  • I have deleted code that I have commented out

Additional Information

  • This PR closes #

};

void loadIntegrations();
}, []);
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be cleaner if components don't do data fetching by themselves.
Instead it should be done in the route loader and passed as props to the component, what do you think?

That will also allow to use the loading (and eventually Suspense) mechanisms.

.min(1, "Please select at least one sensor"),
});

const mqttSchema = z
Copy link
Member

Choose a reason for hiding this comment

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

The mqttSchema contains the ttnConfig?

Can we somehow make this dependent on the available integrations? 🤔 Some sort of generic schema or something..

Comment on lines +96 to +121
// .superRefine((data, ctx) => {
// if (data.mqttEnabled) {
// // Check required fields when enabled is true
// if (!data.url) {
// ctx.addIssue({
// path: ["url"],
// message: "URL is required when MQTT is enabled.",
// code: "custom",
// });
// }
// if (!data.topic) {
// ctx.addIssue({
// path: ["topic"],
// message: "Topic is required when MQTT is enabled.",
// code: "custom",
// });
// }
// if (!data.messageFormat) {
// ctx.addIssue({
// path: ["messageFormat"],
// message: "Message format is required when MQTT is enabled.",
// code: "custom",
// });
// }
// }
// });
Copy link
Member

Choose a reason for hiding this comment

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

Remove?

});

const advancedSchema = z.intersection(mqttSchema, ttnSchema)
// const advancedSchema = z.intersection(mqttSchema, ttnSchema);
Copy link
Member

Choose a reason for hiding this comment

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

remove?

[stepper.current.id]: data,
}
const onSubmit = (data: FormData) => {
console.log("🚀 ~ onSubmit ~ data", data);
Copy link
Member

Choose a reason for hiding this comment

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

lets not keep this :-)

name: text('name').notNull(),
slug: text('slug').notNull().unique(),
serviceUrl: text('service_url').notNull(),
serviceKeyEnvVar: text('service_key_env_var').notNull(),
Copy link
Member

Choose a reason for hiding this comment

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

do we care if the serviceKey is an envVar? It is not used as one by osem right?
So why not keep it simple and call it serviceKey?

slug: text('slug').notNull().unique(),
serviceUrl: text('service_url').notNull(),
serviceKeyEnvVar: text('service_key_env_var').notNull(),
schemaPath: text('schema_path').notNull(),
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it derivable from the serviceUrl?
Your docs state **GET /integrations/schema/{name}**, but I don't fully get the {name} part.
Can a service provide multiple schemas? If so, this can't just be a column and must be a one to many relationship, no?

Comment on lines +20 to +21
MQTT_SERVICE_URL: z.string(),
MQTT_SERVICE_KEY: z.string()
Copy link
Member

Choose a reason for hiding this comment

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

remove for the generic approach?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be part of the docker-compose file too?

Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to check these in?
They are generated files no?

@socket-security
Copy link

socket-security bot commented Feb 11, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​react-router/​serve@​7.12.0 ⏵ 7.13.0991006896100
Updated@​react-router/​express@​7.12.0 ⏵ 7.13.01001006996100
Added@​react-router/​node@​7.13.01001007196100
Added@​rjsf/​utils@​6.2.59910010095100
Added@​rjsf/​core@​6.2.59810010095100
Added@​rjsf/​validator-ajv8@​6.2.510010010095100

View full report

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.

2 participants