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.

Sign up for GitHub

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

Jump to bottom

Feat/graph subscription #552

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Feat/graph subscription #552

wants to merge 3 commits into from

Conversation

tobi-bams
Copy link
Contributor

No description provided.

Evanfeenstra
Evanfeenstra reviewed Jan 17, 2023
View reviewed changes
Copy link
Contributor

@Evanfeenstra Evanfeenstra left a comment

Choose a reason for hiding this comment

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

what is the chat_ids column on the sphinx_graph_suscription exactly for?

I wonder if a table mapping chat_id to subscription_id might be better

@tobi-bams
Copy link
Contributor Author

what is the chat_ids column on the sphinx_graph_suscription exactly for?

I wonder if a table mapping chat_id to subscription_id might be better

The chat_ids column is used to specify what tribe has access to the particular graph_subscription.

Also, a table mapping chat_id to subscription_id is also a good idea I guess

@Evanfeenstra
Copy link
Contributor

Evanfeenstra commented Jan 17, 2023 •
edited
Loading

The chat_ids column is used to specify what tribe has access to the particular graph_subscription.

Also, a table mapping chat_id to subscription_id is also a good idea I guess

The reason i ask is... Chat used to have a contact_ids row, then we ended up making a ChatMembers table, because you can add other information (role, etc). Also you can do more direct queries without needing to JSON.parse() the row.

For example, the way it is now, how could you get "all subscriptions for a chat"?

@tobi-bams tobi-bams force-pushed the feat/graph-subscription branch from 974c2dd to 88cbd49 Compare January 18, 2023 21:47
@kevkevinpal
Copy link
Contributor

merged in #593

@kevkevinpal kevkevinpal closed this Feb 7, 2023
@tobi-bams tobi-bams mentioned this pull request Feb 7, 2023
Merged
@kevkevinpal kevkevinpal reopened this Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Evanfeenstra Evanfeenstra Evanfeenstra left review comments

@kevkevinpal kevkevinpal Awaiting requested review from kevkevinpal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tobi-bams @Evanfeenstra @kevkevinpal