Add 'unique' to model schema - new feature that checks unique constraints when saving/publishing

[hyunnbunt]

[Unique constraints]
A field can be set to have unique constraint.
When saving an entry's draft data, the unique constraint violation is checked (an endpoint is added)
Upon a publish request, the unique constraint violation is checked for all unique fields. (publish endpoint handles this)

[Details]
The unique constraint only applies to the published entries. It does not affect to draft data of an entry. During auto-saving, the violation status of the most recent values entered by the user in the unique fields are checked. If a violation occurs, a message indicating that publishing is not possible is displayed in the input box, and the publish button is disabled. But Saving is not interrupted.
When a publish request is received, the server checks for violations of all unique fields in the requested entry. If a violation occurs, an error is returned, including a list of the violated fields in the error message. The client uses this information to display a message in the input box indicating that publishing is not possible for the violated fields and disables the publish button.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
plasmic-cms-i18n	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 25, 2025 7:51am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
react-email-demo	⬜️ Ignored (Inspect)			Feb 25, 2025 7:51am

[vercel]

@hyunnbunt is attempting to deploy a commit to the Plasmic Team on Vercel.

A member of the Team first needs to authorize it.

[jaslong]

Only reviewed the backend for now, will review frontend later

[jaslong]

What does save return here? It seems to be a different from previously, when it was just row.

[jaslong]

Can't a published row also have draft data?

[hyunnbunt]

Oh, I misunderstood that modifying a published entry would immediately cancel the published status. So, when users modify the entry and request to publish it again, the previous data is overwritten! Got it and fixed it!

[jaslong]

Let's call it /check-unique-fields

[jaslong]

The naming for uniqueChangedFields is specific to the current use case (keyword being "changed"). Also, "fields" is usually used to refer to the schema. I think it's ok to call this data on the server, to be consistent with for example the publish API.

[hyunnbunt]

Thank you for explaining it in detail! I've fixed it!

[jaslong]

Why is row being fetched, but then only the id is being used later?

[jaslong]

Can we return something better than just the field identifier? How about data like:

interface UniqueFieldCheck {
  /** Identifier of schema field. */
  fieldIdentifier: string;
  /** The value that had a violation. */
  value: unknown;
  /** If there are no conflicts. */
  ok: boolean;
  /** Identifiers of rows that conflicted. */
  conflictRowIds: string[];
}

[hyunnbunt]

Got it! Is it okay to add this interface to the same file(DbMgr.ts)?

[jaslong]

Maybe you can change this function to getConflictingCmsRows(data: { [fieldId]: unknown }) that returns CmsRow[]. Then in the routes file you can convert it to UniqueFieldCheck which should live in ApiSchema.

[jaslong]

Can you explain the above lines? I don't understand:

1. What Object.values(publishedRow.data)[0][identifier] ?? "" evaluates to.
2. Why everything is being converted to a string before checking for equality.

It might help to break up this if into multiple if statements and use more variables.

[jaslong]

1. Handles locales? Nah, can either be localized or unique, not both.
2. Should use some sort of deep equality check (look at lodash.equals).
3. Handle empty data during comparison. Make a normalizeData function to handle these kind of weird cases, then in the future we can apply this function to the rest of the code to make sure data integrity.

[jaslong]

Slightly weird to use both foreach and for loop in the same function. It would be better to stick with one style. Generally we try to be functional, so I think it would be best to do something like:

return Object.entries(fields)
  .map(([identifier, value], () => {
    const conflictRowIds = publishedRows
      .filter(row => /* whether row conflicts with value */);
    if (conflictRowIds.length > 0) {
      return {
        identifier,
        value,
        conflictRowIds
      }
    }
  });

[jaslong]

Try to get rid of these states and use the rules framework. Here's the docs: https://ant.design/components/form#rule

[jaslong]

What happens if you check the unique switch, then change the type to a list or object?

Similar for localized in the future.

[jaslong]

uniqueFields =  table.schema.fields
        .filter((field) => field.unique)
        .map((field) => field.identifier)

OR

const uniqueFields = useMemo(() => {
  return table.schema.fields
        .filter((field) => field.unique)
        .map((field) => field.identifier)
}, [table]);

[jaslong]

Can't assume there is only 1 element in the changed values

[jaslong]

Use 409

The HTTP 409 Conflict status code indicates that a request could not be completed due to a conflict with the current state of the target resource, often occurring during operations like file uploads or updates. This error typically requires resolving the conflict before the request can be successfully processed.

[jaslong]

Before assuming that the error is about uniqueness, you need a way to guarantee that it is a uniqueness issue, because we might have other kinds of errors, such as auth error, etc.

Make a new error type in ApiSchema.

export type UniqueViolationError = {
  type: "unique-violation",
  violations: UniqueViolation[],
}

// this is called a typeguard
export function isUniqueViolationError(err: unknown): err is UniqueViolationError {
  return typeof err === "object" && err && err.type === "unique-violation"
}

catch (err) {
  if (isUniqueViolationError(err)) {
    setViolations(...)
  } else {
   throw err;
  }

[jaslong]

I don't think checkUniqueness should be in performSave. Rather, you should implement checkUniqueness as a parallel process.

[jaslong]

Remove try/catch before merging

[jaslong]

Change return type of checkUnique to make this simpler.

notValidUniqueFields = { a: UniqueFieldCheck, b: UniqueFieldCheck }
checkedNotValid = await api.checkUnique(...) // { b: true, c: false }
Object.entries(checkedNotValid).forEach(([identifier, check]) => {
  if (check.ok) {
    notValidUniqueFields.remove(identifier);
  } else {
    notValidUniqueFields.set(identifier, check);
  }
});

[jaslong]

setState(prev => {
  return { ...prev, newStuff }
})

// type { "uniqueField": "a" }
// start request unique check for { "uniqueField": "a" }
// type { "uniqueField": "ab" }
// finish request unique check for { "uniqueField": "a" } <- this should be ignored
// start request unique check for { "uniqueField": "ab" }'

type UniqueFieldStatus {
  value: string
  status: "not started" | "pending" | "ok" | "violation"
  conflictEntryIds: string[]
}

// {
//    foo: { value: "abc", status: "not started" }
// }
const [uniqueFieldStatus, setUniqueFieldStatus] = setState<{ [fieldId: string]: UniqueFieldStatus>({});
...
onValuesChanged(values) {
  setUniqueFieldStatuses(prevStatuses => {
    const copy = { ...prevStatuses };
    values.forEach(([fieldId, value]) => {
      if (copy[fieldId].value === value) {
        // do nothing, because we already have the same value
      } else  {
        copy[fieldId] = value;
      }
    })
    return copy;
  });
}

checkUniqueness() {
  const checkedNotValid = await api.checkUnique(row.id, opts);
  setUniqueFieldStatus(prevStatus => {
    // left as reader's exercise
  });
}