馃彈 Migrated unsplash setting to up to date convention #12632
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
tpatel
suggested changes
Feb 12, 2021
There was a problem hiding this comment.
I would either add irreversible: true in the config (that would require an update to the helper) or return Promise.reject(); in down() like in
Ghost/core/server/data/migrations/utils.js
Line 261 in 873d21d
I think it's better than doing nothing so that we can't run this migration twice.
|
@tpatel good spot! Changed the migration 馃槈 |
2 tasks
allouis
requested changes
Feb 12, 2021
core/server/data/migrations/versions/4.0/10-refactor-unsplash-setting.js
Outdated
Show resolved
Hide resolved
naz
force-pushed
the
unsplash-setting-migration
branch
3 times, most recently
from
cbbf6a8
to
c9795c5
Compare
naz
force-pushed
the
unsplash-setting-migration
branch
from
f0777d7
to
7cfb38c
Compare
allouis
approved these changes
Feb 15, 2021
daniellockyer
approved these changes
Feb 16, 2021
naz
force-pushed
the
unsplash-setting-migration
branch
from
7cfb38c
to
0bb7be3
Compare
|
@daniellockyer @tpatel @allouis I've updated the migration here to follow idempotence rule. The commit which fixes is is this one - 0bb7be3 |
tpatel
approved these changes
Feb 17, 2021
allouis
approved these changes
Feb 17, 2021
daniellockyer
approved these changes
Feb 17, 2021
refs TryGhost#10318 - JSON object format used in previous "unsplash" setting was considered an anti-pattern. Flat structure was extracted out of the "unsplash.isActive" JSON. - The naming convention uses `amp` as a precedent (TryGhost/Product#331 (comment))
naz
force-pushed
the
unsplash-setting-migration
branch
from
0bb7be3
to
7298f70
Compare
refs TryGhost#10318 - API changes introduced: canary/v4 Admin API GET /settings/ (browse) + "unsplash" present in response as boolean value GET /settings/:settingName (read) + "unsplash" present in response as boolean value PUT /settings/ (edit) + "unsplash" updates setting, accepts ONLY boolean format v3 Admin API GET /settings/ (browse) + "unsplash" present in response with object value GET /settings/:settingName (read) + "unsplash" present in response with object value PUT /settings/ (edit) + "unsplash" updates setting, accepts either boolean or object formats v2 Admin API GET /settings/ (browse) + "unsplash" present in response with object value GET /settings/:settingName (read) + "unsplash" present in response with object value PUT /settings/ (edit) + "unsplash" updates setting, accepts object format
no issue - The migration didn't want to go away after rebases combined with renames
refs TryGhost#10318 - Previous version of migration was not following the idempotence rule of migrations
naz
force-pushed
the
unsplash-setting-migration
branch
from
7298f70
to
394d204
Compare
naz
added a commit
to TryGhost/Admin
that referenced
this pull request
Feb 17, 2021
refs TryGhost/Ghost#12632 - The `unsplash` setting has been migrated to a new format - plain boolean value. This is why it's no longer needed to do any serialization dances
naz
added a commit
to TryGhost/Admin
that referenced
this pull request
Feb 18, 2021
refs TryGhost/Ghost#12632 - The `unsplash` setting has been migrated to a new format - plain boolean value. This is why it's no longer needed to do any serialization dances
naz
added a commit
to TryGhost/Admin
that referenced
this pull request
Feb 18, 2021
refs TryGhost/Ghost#12632 - The `unsplash` setting has been migrated to a new format - plain boolean value. This is why it's no longer needed to do any serialization dances
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
splits #12615 into "slack" and "unsplash" parts
refs closes TryGhost/Team#331
refs #10318
Note to reviewers: the part that needs thorough review is the migration located in 10-refactor-unsplash-setting.js file. The rest is optional if you feel like having a look 馃槈
TODO:
Objectreturn type in v4 apiChange/compatibility table for this PR:
importer
unsplash鉁旓笍 import from both boolean AND objectcanary/v4
GET /settings/(browse)unsplash鉁旓笍 present in response withbooleanvalueGET /settings/:settingName(read)unsplash鉁旓笍 present in response withbooleanvaluePUT /settings/(edit)unsplash鉁旓笍 updates setting, accepts a booleanv3 Admin API
GET /settings/(browse)unsplash鉁旓笍 present in response withObjectvalueGET /settings/:settingName(read)unsplash鉁旓笍 present in response withObjectvaluePUT /settings/(edit)unsplash鉁旓笍 updates setting, accepts either boolean or objectv2 Admin API
GET /settings/(browse)unsplash鉁旓笍 present in response with object valueGET /settings/:settingName(read)unsplash鉁旓笍 present in response with object valuePUT /settings/(edit)unsplash鉁旓笍 updates setting, accepts object