feat: Add pipeline ui fields to plugin config #18384
Conversation
tiina303
force-pushed
the
pipeline-ui-plugin-config-fields
branch
from
a0a53f7 to
f00d83d
Compare
tiina303
force-pushed
the
pipeline-ui-plugin-config-fields
branch
from
c359772 to
630331d
Compare
| migrations.AddField( | ||
| model_name="pluginconfig", | ||
| name="deleted", | ||
| field=models.BooleanField(blank=True, default=False, null=True), |
There was a problem hiding this comment.
I don't think we should set null here. default=False should be enough for the migration I think? I'm imagining this roughly translates to: ALTER TABLE posthog_pluginconfig ADD COLUMN deleted DEFAULT FALSE which means PG will store FALSE for all rows at the time of the migration, without needing a rewrite.
There was a problem hiding this comment.
It could be that django complains though, but from Postgres' side this should be doable without null.
There was a problem hiding this comment.
exactly I was worried about the migration, would be more than happy to change it to default=False if that's safe
There was a problem hiding this comment.
tiina303
force-pushed
the
pipeline-ui-plugin-config-fields
branch
from
0079985 to
91dd621
Compare
There was a problem hiding this comment.
You probably have another PR for the plugin-server side: we'll want to make sure we don't pull deleted configs. I suggest:
- having the django model patch enabled=false on deletion
- still adding the
and not deletedto the plugin-server query in case there's a regression in the orm layer
Problem
It's possible to have multiple plugin configs for the same plugin. So we'll have name and description for users in the ui similar to how experiments and feature flags have those fields.
We want to allow users to also remove plugin configs from the ui to clear up their view, but to support undo and safety we don't want to hard delete them.
Related thread: https://posthog.slack.com/archives/C0374DA782U/p1698956443520489
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?
went to apps page locally after setting name and description for hello world app, saw this in network panel