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

Invite logic needs updating in knex #86

Closed
sreynen opened this issue Aug 1, 2017 · 4 comments
Closed

Invite logic needs updating in knex #86

sreynen opened this issue Aug 1, 2017 · 4 comments
Assignees
Labels
A-database Area: DB related changes including but not limited to knex, and ORM like structures C-bug Type: Bug

Comments

@sreynen
Copy link
Contributor

sreynen commented Aug 1, 2017

Currently the invite link is going to /invite/null because it's not getting the ID back on creation. But even if it does get the ID back, we need to use something other than the ID for the URL, because IDs in PostgreSQL are not obscure hashes like they are in RethinkDB, they're easily guessable numbers.

@sreynen sreynen added C-bug Type: Bug A-database Area: DB related changes including but not limited to knex, and ORM like structures labels Aug 1, 2017
@sreynen sreynen self-assigned this Aug 1, 2017
@hiemanshu
Copy link
Collaborator

RethinkDB used UUIDs as the primary key, you can have postgresql (using pgcrypto extension) or sqlite (using a plugin) generate and use UUIDs as their primary key. Postgresql has a uuid type, Sqlite can use string or binary blob to store UUIDs (binary blobs are faster, but less human readable)

@schuyler1d
Copy link
Collaborator

it looks like knex queries have uuid for each query, as well, so maybe we can use those to fill those in. might be worth trying to add it in rethink-knex-adapter (but as a separate field, in most cases). invite though could be a primary field.

@sreynen
Copy link
Contributor Author

sreynen commented Aug 3, 2017

FYI https://github.com/MoveOnOrg/Spoke/tree/wip-86-invite-links has work in progress on this.

@sreynen
Copy link
Contributor Author

sreynen commented Aug 4, 2017

This should be resolved now.

@sreynen sreynen closed this as completed Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-database Area: DB related changes including but not limited to knex, and ORM like structures C-bug Type: Bug
Projects
None yet
Development

No branches or pull requests

4 participants