-
Notifications
You must be signed in to change notification settings - Fork 324
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: make all templates free #9503
Conversation
await client.end() | ||
} | ||
|
||
const PREV_FREE_TEMPLATES = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 This list is incomplete, after running the down migration I have 75 non-free templates. On production there are only 20. Running
select * from "MeetingTemplate" where "isFree" = false and "orgId" = 'aGhostOrg'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's because the first query in the down migration didn't check where orgId is aGhostOrg
, so all of the custom templates were set to non-free.
I've updated the down migration and checked staging for the list of free template ids.
packages/server/postgres/migrations/1709551949071_makeAllTemplatesFree.ts
Show resolved
Hide resolved
await client.end() | ||
} | ||
|
||
const PREV_FREE_TEMPLATE_IDS = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 We're still missing more free templates: https://github.com/ParabolInc/parabol/blob/master/packages/server/postgres/migrations/1698831891828_moreFreeRetroTemplates.ts
There are probably others as well. I'd rather skip the down migration altogether than making more templates paid than necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 This can now be removed
await client.end() | ||
} | ||
|
||
const PREV_FREE_TEMPLATE_IDS = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 This can now be removed
Fix #9472
I suggest that we don't remove all of the template limit logic as it's possible that we will want to implement template limits again in the near future.
So, we should just set
isFree
to true for all meeting templates.When I run
yarn pg:migrate up
I get the following error locally:This migration is almost identical to this one, so I assume it's a problem with my local set-up. It's referring to Rethinkdb when the table is in PG. Please let me know if you recognise the error. If not, I'll keep digging into it.
To test