estuary / ui

A web based UI to assist in working with Estuary Flow
https://dashboard.estuary.dev/
Other
12 stars 1 forks source link

Can not remove repeated email subscribers from notifications #1223

Closed dyaffe closed 1 week ago

dyaffe commented 4 weeks ago

Bug If you add the same email address multiple times, in notifications, it isn't possible to remove the repeats.

Message me for reproduction steps.

kiahna-tucker commented 3 weeks ago

If duplicate notification subscriptions exist in the database, it is not currently possible to remove a single duplicated email in the client. To de-dupe an email subscription using the client today, you would need to first remove all instances of that email, submit the changes, then add the email back to the list.

The client guards against the introduction of duplicate email subscriptions fairly effectively. In order to enable client-side removal of duplicate emails, the database table key and the date the row was created needs to be retained in the client. The date an email subscription was created should factor into the deletion process to preserve the integrity of the database table. Preferably, an RLS policy would be introduced that would prevent duplicate subscriptions from being defined, especially given one will be required the moment the catalog prefix constraint is relaxed.

travjenkins commented 3 weeks ago

We should update the DB for sure. Opened https://github.com/estuary/flow/issues/1571 for this.

But I think the UI could remove the emails without extra details. We should order the delete and then limit it. Gets a little funky but could work.

kiahna-tucker commented 1 week ago

Travis concluded that the code required to identify and remove duplicate, existing email subscriptions in the client is too complex. He would prefer to run a migration to remove all existing, duplicate email subscriptions in the alert_subscriptions table and introduce an RLS policy (or something on the database's end) that blocks the creation of a duplicate subscription (as advised in my initial comment).

The client can attempt to prevent the creation of duplicate alert subscriptions but, as initially advised, the bulk of the protection should be woven into the database.

kiahna-tucker commented 1 week ago

Closing as the remainder of the work needs to be handled on the backend. See the linked flow issue for to monitor progress.