Skip to content

Conversation

@eric-foster-angi
Copy link

@eric-foster-angi eric-foster-angi commented Jun 11, 2025

Summary

Implements fix for issue: #1166

When updating the valid Content Types for a reference Content Model field, you cannot just insert or add a single entry as validations will completely write over the existing entries. This requires copying and pasting the list which introduces the risk that someone might do it wrong and wipe out needed values.

Instead, we should be able to just add/insert a new value to the array, reducing risk and headache.

Description

implements a new method addItemsValidation that allows you to update existing validations on items without completely writing over the existing values.

Motivation and Context

Given an existing model defined as:

  testModel
    .createField('relatedEntries')
    .name('Related Entries')
    .type('Array')
    .items({
      type: 'Link',
      linkType: 'Entry',
      validations: [{ linkContentType: ['contentModelA', 'contentModelB']}]
    })

Today if we wanted to add contentModelC to this list of validations, we must redefine the entire array:

  testModel
    .editField('relatedEntries')
    .items({
      type: 'Link',
      linkType: 'Entry',
      validations: [{ linkContentType: ['contentModelA', 'contentModelB', 'contentModelC']}]
    })

Instead it would be desirable to be able to add a single entry:

  const testModel = migration.editContentType('testModel')
  testModel
    .editField('relatedEntries')
    .addItemsValidation([{ linkContentType: ['contentModelC'] }])

Todos

None

Screenshots

skills-validation-atleast-1 related-entries-validations-after-update test-model-fields new-content-models-created (if appropriate):

@eric-foster-angi eric-foster-angi requested a review from a team as a code owner June 11, 2025 14:10
@Syddhantt
Copy link

SUPPORT. This is great.

@Niko-Berry-Contentful
Copy link

Hey. Thank you for making this commit. Our team is taking a look at it and will let you know if we have any questions.

@ethan-ozelius-contentful
Copy link
Contributor

ethan-ozelius-contentful commented Jun 16, 2025

Just a quick update: still taking a look at this, will have more concrete feedback/questions/approvals in the next 2 weeks, hopefully sooner.

In the meantime I'll run the circle CI tests.

Thanks for the patience.

@ethan-ozelius-contentful
Copy link
Contributor

Looks like there might be some failing tests?

 // npm run test:unit
  337 passing (538ms)
  1 pending
  3 failing

  1) FieldAddItemsValidationIntent
       when adding validations to array field items
         adds validations to existing items validations:
     TypeError: callsite.getFileName is not a function
      at Object.addItemsValidation (src/lib/migration-steps/action-creators.ts:739:29)
      at Context.<anonymous> (test/unit/lib/intent/field-add-items-validation.spec.ts:12:74)
      at processImmediate (node:internal/timers:476:21)

  2) FieldAddItemsValidationIntent
       when adding validations to array field items
         creates items validations array if it does not exist:
     TypeError: callsite.getFileName is not a function
      at Object.addItemsValidation (src/lib/migration-steps/action-creators.ts:739:29)
      at Context.<anonymous> (test/unit/lib/intent/field-add-items-validation.spec.ts:51:74)
      at processImmediate (node:internal/timers:476:21)

  3) FieldAddItemsValidationIntent
       when adding validations to array field items
         creates items object if it does not exist:
     TypeError: callsite.getFileName is not a function
      at Object.addItemsValidation (src/lib/migration-steps/action-creators.ts:739:29)
      at Context.<anonymous> (test/unit/lib/intent/field-add-items-validation.spec.ts:88:74)
      at processImmediate (node:internal/timers:476:21)

Exited with code exit status 3
// npm run lint


> [email protected] lint
> eslint 'examples/**/*.js' 'test/**/*.js' 'src/**/*.js' && tslint --project tsconfig-dev.json --config tslint.json -e '**/*.js'

strict-type-predicates does not work without --strictNullChecks
/home/circleci/project/test/unit/lib/actions/field-add-items-validation.spec.ts:89:7
ERROR: 89:7   no-unused-expression  unused expression, expected an assignment or function call

/home/circleci/project/test/unit/lib/intent/field-add-items-validation.spec.ts:117:7
ERROR: 117:7  no-unused-expression  unused expression, expected an assignment or function call
// npm run test:integration
  47 passing (57s)
  1 failing

  1) the migration
       updates items validation for existing field:
     AssertionError: expected [ { linkContentType: [ …(3) ] } ] to deep include { linkContentType: [ 'contentModelC' ] }
      at Context.<anonymous> (test/integration/migration.spec.js:1466:59)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

const fields = ct.fields
const field = fields.getField(this.getFieldId())

if (!field.items) {
Copy link
Contributor

Choose a reason for hiding this comment

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

field would only have a items property if it is of type reference or media. So we might want to add one more check here to make sure we don't unnecessarily add the items field to a field where it wouldn't belong. Something like:

if (!field.items && ['Array', 'Link'].includes(field.type)) {

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.

4 participants