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

feat(auth): tenants table v1 #3133

Merged
merged 3 commits into from
Dec 2, 2024
Merged

Conversation

njlie
Copy link
Contributor

@njlie njlie commented Nov 26, 2024

Changes proposed in this pull request

  • Add tenants table in migration for backend database
  • Add ObjectionJS model.ts file to model tenants table schema

Context

Closes #3113.

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Make sure that all checks pass
  • Bruno collection updated (if necessary)
  • Documentation issue created with user-docs label (if necessary)
  • OpenAPI specs updated (if necessary)

@github-actions github-actions bot added type: source Changes business logic pkg: auth Changes in the GNAP auth package. labels Nov 26, 2024
Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit 68efdeb
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/67450f9e1afb810008b3be3b

@njlie njlie changed the base branch from main to 2893/multi-tenancy-v1 November 26, 2024 19:03
Comment on lines +11 to +12
table.timestamp('createdAt').defaultTo(knex.fn.now())
table.timestamp('updatedAt').defaultTo(knex.fn.now())
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these generated in this table, or passed over from backend?
Should we also include deletedAt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would say that yes, this createdAt and updatedAt needs to be generated in this table since it is different entity. Bot somehow, still connected with the one in backend. with createdAt we get info when was the entity created in that database, not when it was created in the backed database. What do you think ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Generating the timestamps in this table should be fine, that's consistent with what we have everywhere else.
But we should include deletedAt column, which should just be passed in from the deleteTenant GraphQL API call.

@njlie njlie force-pushed the nl/3113/auth-tenants-table branch from dc10e8f to b9c2a72 Compare December 2, 2024 16:50
@njlie njlie requested a review from mkurapov December 2, 2024 16:58
@njlie njlie merged commit ea7e660 into 2893/multi-tenancy-v1 Dec 2, 2024
24 of 36 checks passed
@njlie njlie deleted the nl/3113/auth-tenants-table branch December 2, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: auth Changes in the GNAP auth package. type: source Changes business logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Multi-Tenant] auth tenants table
4 participants