MemberJunction / MemberJunction/MJ
CodeGen should warn when a field has ValueListType='List' but no EntityFieldValue rows
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
CodeGen can set `EntityField.ValueListType='List'` on a field that has **no `EntityFieldValue` rows**. That combination describes a field where *no value is legal*, which should never exist — it means the metadata is broken. Nothing catches it at generation time, so it surfaces (if at all) at runtime.
Raised by @AN-BC in review of PR #3972:
> This is also something that CodeGen _could_ check for - a column that has ValueListType == List and has no values. Since the vast majority of the situations that drive the setting of ValueListType come from CodeGen parsing check constraints this is, again, extraordinarily unlikely.
- Status: OPEN
- Layer: `@memberjunction/codegen-lib` — `ManageMetadataBase.manageEntityFieldValuesAndValidatorFunctions` / `applyValueListConfig`
## Why it is worth a build-time check even though it is unlikely
The runtime half already landed: `EntityFieldInfo.ValueIsPermittedByValueList()` (#3969, PR #3972) permits everything for such a field and emits a `LogError` naming the entity and field, latched once per `EntityFieldInfo`. That is the right runtime behaviour — refusing every value would fail every save on the field — but it is a *late* and *conditional* report: it needs the field to be validated in a running process before anyone learns the metadata is wrong.
CodeGen already has everything needed to say it at generation time, where it is a build-time warning against the definitive metadata rather than a log line someone has to notice in production.
## Where it can arise
1. **`ValueListType` set, then the values removed.** `syncEntityFieldValues` deletes `EntityFieldValue` rows no longer present in the parsed list, but the `ValueListType='List'` update is a separate statement (`manage-metadata.ts` ~5270) — nothing re-evaluates the type when the list empties.
2. **A CHECK constraint edited into a shape the parser no longer matches.** `parseCheckConstraintValues` returns null for anything that is not exactly a disjunction of quoted equalities, and the constraint then routes to the AI-validator path — leaving an earlier `ValueListType='List'` in place with a list that is now stale or emptied.
3. **A hand-authored or soft (`applyValueListConfig`) list whose `PossibleValues` is empty.** `SoftFieldValueListConfig.PossibleValues` is not checked for emptiness before the type is applied.
4. **Metadata edited directly**, which is how it would most plausibly happen in a real deployment.
## Suggested fix
After `manageEntityFieldValuesAndValidatorFunctions` completes, one query over the metadata it just wrote:
```sql
SELECT e.Name AS EntityName, ef.Name AS FieldName
FROM __mj.EntityField ef
JOIN __mj.Entity e ON e.ID = ef.EntityID
WHERE ef.ValueListType = 'List'
AND NOT EXISTS (SELECT 1 FROM __mj.EntityFieldValue v WHERE v.EntityFieldID = ef.ID);
```
Report each hit as a CodeGen warning naming the entity and field, with the two remedies: re-derive the list, or set `ValueListType='None'` if the field is not meant to be constrained. A warning rather than an error, since the runtime fails open and this should not break a build.
Two decisions for whoever picks it up:
1. **Warn or auto-correct?** CodeGen could set `ValueListType='None'` itself, since `List`-with-no-values is never meaningful. That is tempting but it silently discards the *intent* that someone marked the field as constrained — a warning keeps the human in the loop, which matches how the runtime handles it (permit, but say so).
2. **Should the same pass check the inverse** — `EntityFieldValue` rows present while `ValueListType='None'`? That is the more common drift direction and equally cheap to detect in the same query.
## Related
- #3969 / PR #3972 — the runtime `LogError` this would pre-empt.
- #3978 — numeric/bit `IN (…)` lists produce no value list at all (a different parser gap in the same function).
- #3985 — eight MJ core `List` fields whose SQL default is not in their own value list; a related "the metadata disagrees with itself" check that CodeGen is also well placed to catch.
Contributor guide
Research direction
Start in @memberjunction/codegen-lib's manage-metadata.ts, especially manageEntityFieldValuesAndValidatorFunctions, syncEntityFieldValues, and applyValueListConfig. Trace the metadata writes, then add the post-generation check so each List field without EntityFieldValue rows produces a warning naming its entity and field; preserve the suggested remedies and do not fail the build.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- sql, typescript
- Domain
- databases, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 72/100