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

New Action to transform entries to another content type #143

Merged
merged 42 commits into from
Jan 2, 2019
Merged

New Action to transform entries to another content type #143

merged 42 commits into from
Jan 2, 2019

Conversation

aKzenT
Copy link
Contributor

@aKzenT aKzenT commented Oct 19, 2018

Summary

This PR extends the available migrations with a new "transformEntriesToType"-transform that allows migrations from one content type to another as discussed in #113

Description

A new action was added to support the transformation. Various enhancements to the OfflineAPI as well as the Fetcher were needed to implement the features.

Motivation and Context

See #113

Todos

  • Basic functionality working
  • Option to remove the source item after transformation
  • Option to publish/not publish or keep the state of the original entry
  • Option to change all references to the original entry so that they point to the new entry instead
  • Additional tests

@aKzenT
Copy link
Contributor Author

aKzenT commented Dec 21, 2018

@TimBeyer can you take a look at the changes? We are planning to do several more migrations in the beginning of January, which will further test this new migration type.

@Khaledgarbaya Khaledgarbaya requested a review from TimBeyer January 2, 2019 09:09
TimBeyer
TimBeyer previously approved these changes Jan 2, 2019
Khaledgarbaya
Khaledgarbaya previously approved these changes Jan 2, 2019
@Khaledgarbaya
Copy link
Contributor

Hey @aKzenT,

This is ready to merge. can you please run npm run lint and fix the linting issues and push again.

After that, I will merge this PR.

@GordonApplepie GordonApplepie dismissed stale reviews from Khaledgarbaya and TimBeyer via d3a9ff3 January 2, 2019 10:51
@GordonApplepie
Copy link
Contributor

happy new year all together. @Khaledgarbaya I fixed the linter errors as requested. I am a bit unsure of the warning "no-unused-variable is deprecated" since this is a linter settings issue. I would ignore this for now?

@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 2, 2019

@GordonApplepie you can ignore it for now.
I think this is now a Typescript setting and so the linter option has been deprecated.

@Khaledgarbaya Khaledgarbaya merged commit f5b3595 into contentful:master Jan 2, 2019
@phoebeschmidt
Copy link
Contributor

🎉 This PR is included in version 0.16.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@GordonApplepie
Copy link
Contributor

🎉🎉🎉 @aKzenT

thanks to all

@Khaledgarbaya
Copy link
Contributor

Hey @aKzenT and @GordonApplepie we would like to send you some thank you swags for your contributions.
Can you ping me at this email khaled[at]contentful.com with your addresses so we can ship them to you?

@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 3, 2019

Hey @aKzenT, I just realized that we forgot to add a README.md addition for this change.
Do you think you could still open a small PR for that so that this is documented for everyone? 🙂

@GordonApplepie
Copy link
Contributor

Hi @TimBeyer, I will look after that now. Sadly I found a problem due to recent updates in the master branch regarding the "fetcher.ts". There is a new filter implementation that actually prevents to load the complete list of entries which we need to update the references.

I will describe it again in the pull request. Editing the readme file now.

@jarkkosyrjala
Copy link
Contributor

jarkkosyrjala commented Jan 4, 2019

I'm getting "Validation failed" error when trying to use shouldPublish: 'preserve':

"string" is not a valid type for the content transformation property "shouldPublish". Expected "boolean".

Using version 0.16.3

Full example of the migration file:

module.exports = function (migration) {
  const fields = ['field1','field2','field3']
  const contentType = 'exampleContentType'
  const sourceLocale = 'en'
  const targetLocale = 'en-US'
  migration.transformEntries({
    contentType: contentType,
    from: fields,
    to: fields,
    shouldPublish: 'preserve',
    transformEntryForLocale: function (fromFields, currentLocale) {
      if (currentLocale === targetLocale) {
        return fields.reduce(function(result, fieldName) {
          if (fromFields[fieldName] && fromFields[fieldName][sourceLocale]) {
            result[fieldName] = fromFields[fieldName][sourceLocale]
          }
          return result;
        }, {});
      } else {
        // Ignore other locales
        return;
      }
    }
  });
};

Should I create a separate Issue about this?

@GordonApplepie
Copy link
Contributor

GordonApplepie commented Jan 4, 2019

@jarkkosyrjala I think you are mixing two different actions here "transformEntries" is not "transformEntriesToType". "preserve" is only available in the last one.

@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 4, 2019

@GordonApplepie if I saw it correctly, then 'preserve' was also implemented for the normal transformEntries in this PR. Probably it just wasn't added to the validations.

@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 4, 2019

@GordonApplepie what I updated in the fetcher was to always iterate through all pages for all paginated types, and to exclude archived entries from the transform.
Which one of those is causing you trouble?

Our assumption was that since archived entries are basically soft deleted, they most likely should not be part of any transform or derive action.

@GordonApplepie
Copy link
Contributor

#163

@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 4, 2019

@GordonApplepie I made a PR that fixes the issue in #164

@jarkkosyrjala
Copy link
Contributor

@GordonApplepie if I saw it correctly, then 'preserve' was also implemented for the normal transformEntries in this PR. Probably it just wasn't added to the validations.

Made a PR that fixes this: #165

Khaledgarbaya pushed a commit that referenced this pull request Jan 9, 2019
…o boolean values (#165)

## Summary

shouldPublish should accept 'preserve' in addition to boolean values

## Description

Add 'preserve' as a valid option to validations for shouldPublish field

## Motivation and Context

#143 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants