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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data & Fixture Migrations v005 #6628

Merged
merged 8 commits into from Apr 15, 2016
Merged

Data & Fixture Migrations v005 #6628

merged 8 commits into from Apr 15, 2016

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Mar 22, 2016

This is a brand new PR for pushing migrations that need to happen in 005 馃摎 馃殌

Back in #6302, we had migrations that were auto-calculated from the schema / fixtures files. This was easier to do at face value, but felt risky because it was impossible to test and verify that changes would work correctly. The new version requires us to write more code, but gives us more guarantee that the upgrades will work now and in the future.

From here on, each change to the schema or fixtures needs to follow some rules:

  • changes to schema or fixtures must be accompanied by a matching upgrade task
  • all upgrades should have a check to ensure that they only run if they are needed
  • all migration code should have 100% unit test coverage

@ErisDS ErisDS changed the title Data & Fixture Migrations v005 [WIP/HOLD] Data & Fixture Migrations v005 Mar 22, 2016
@ErisDS ErisDS force-pushed the migration-005 branch 4 times, most recently from 9757294 to 4a8cf11 Compare March 26, 2016 14:36
@acburdine acburdine mentioned this pull request Apr 4, 2016
4 tasks
@ErisDS
Copy link
Member Author

ErisDS commented Apr 10, 2016

@sebgie in the last migration here, I've added a new ghost-scheduler client, which is intended to be used for calling back to the scheduling API #6399 at the correct time. This client, unlike the existing two, is going to be confidential rather than public, I believe?
To indicate this, I've given it a type of wa rather than ua. This comes from the spec here, which is the only reference I could find to a ua type. Is this what you expected? Is it worth us adding an isIn validation to type in the schema to enumerate the allowed values?

@sebgie
Copy link
Contributor

sebgie commented Apr 11, 2016

Is this what you expected?

Yes, the types are meant to reflect the specification. Tbf, na for native application could look like not available?

name short
user-agent ua
web application wa
native application na

@ErisDS
Copy link
Member Author

ErisDS commented Apr 11, 2016

Fair point, I was just following along from what was there, I don't think there's a particular reason to keep them so short - ua makes sense for user agent, but we could make the other two clearer?

name short
user-agent ua
web application web
native application native

@ErisDS ErisDS force-pushed the migration-005 branch 2 times, most recently from eef614a to 9ee52ce Compare April 14, 2016 14:21
refs TryGhost#6301

- bump the default version & update corresponding test
- add empty task folders for 005 data & fixture migrations
- update tests to cover the new 005 upgrades
refs  TryGhost#6301

- move the temporary `fixClientSecret` function from migration.init into being a proper fixture migration task
- update the tests accordingly
refs TryGhost#6301

- column is not used and we'll be adding a visibility column to serve the intended purpose
refs TryGhost#6301, TryGhost#6165

- visibility is added as a new column on posts, tags and users.
- has a relevant default value for each table
refs TryGhost#6301, TryGhost#6255

- new, extra-long, column for storing mobiledoc content format
refs TryGhost#6301, TryGhost#6534

- adds facebook and twitter columns, which should contain urls
refs TryGhost#6301, TryGhost#6399

- new scheduler client will be used by any web app that handles time and calls back to the scheduling API at the right time
- new scheduler client will need to be confidential, rather than public, hence the 'web' type instead of 'ua'
- adds validation to client type that it must have a type of 'ua', 'web', or 'native'
refs TryGhost#6301, TryGhost#4176

Add migration for:
- 5 new client permissions
- 15 relations between the admin, editor & author role and the 5 new permissions
- updates to tests to show that permissions get updated properly
@ErisDS ErisDS changed the title [WIP/HOLD] Data & Fixture Migrations v005 Data & Fixture Migrations v005 Apr 15, 2016
@sebgie sebgie merged commit 817a302 into TryGhost:master Apr 15, 2016
@ErisDS ErisDS deleted the migration-005 branch April 25, 2016 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants