Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(realtime): enable realtime publication for signature requests #242

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

Jipperism
Copy link
Contributor

@Jipperism Jipperism commented Feb 4, 2025

  • important note: subscribing to a table that doesn't have realtime publications enabled, disables existing subscriptions to all tables 🥴

Introduced in 70c8131. I think it would be good if we can find a way to double check at runtime whether all tables that are being subscribed to have realtime publications enabled.

- important note: subscribing to a table that doesn't have realtime publications enabled, disables subscriptions to all tables 🥴
Copy link

github-actions bot commented Feb 4, 2025

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 25.35% (🎯 25%) 1097 / 4327
🟢 Statements 25.35% (🎯 25%) 1097 / 4327
🟢 Functions 61.05% (🎯 61%) 58 / 95
🟢 Branches 72.28% (🎯 72%) 180 / 249
File CoverageNo changed files found.
Generated in workflow #49 for commit 7d71aa4 by the Vitest Coverage Report Action

@Jipperism Jipperism self-assigned this Feb 4, 2025
@Jipperism Jipperism changed the title feat(realtime): enable realtime publication for signature requests fix(realtime): enable realtime publication for signature requests Feb 4, 2025
@bitbeckers bitbeckers added the bug Something isn't working label Feb 5, 2025
@bitbeckers
Copy link
Collaborator

@Jipperism to your point on checking for publications. Why not enable them for all tables?

https://supabase.com/docs/guides/database/replication#create-a-publication

@Jipperism
Copy link
Contributor Author

@bitbeckers interesting idea. Not 100% sure if that would solve the problem, we'd have to verify if that is for all tables at the time the migration is run, or all tables from now on. If so, yeah why not?

@bitbeckers
Copy link
Collaborator

Looks like we can drop the publication and recreate it for all tables in a single migration: https://supabase.com/docs/guides/database/replication#recreate-a-publication

@Jipperism
Copy link
Contributor Author

Yes, but that would then have to be added to every migration that creates a new table, right?

@bitbeckers
Copy link
Collaborator

Yes. Would a solution be to run a GHA after the migration that gets all entries from publication table and a list of all tables and finds the diff?

@Jipperism
Copy link
Contributor Author

@bitbeckers is the idea that it then errors if not all tables are in the publication? Or it then creates a migration where we add all missing tables to the realtime publication?

Different approach: wrapper method for subscribing that checks the table being subscribed to against all tables in the realtime publication. That would solve the problem at startup time, so you'd see it before pushing. I think both would be fine, the wrapper method would keep the migrations cleaner.

@bitbeckers
Copy link
Collaborator

bitbeckers commented Feb 7, 2025

@Jipperism for the rest of the discussion, let's not get bikeshed here: #246

edit: sorry I though I already approved

@Jipperism Jipperism merged commit b7c9587 into develop Feb 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants