refactor(perms): tighten comments and drop dead code

Cleanup pass on the issue-37 permission-filtering work — no behaviour
changes, all 180 unit tests still pass, browser-verified across admin,
restricted-with-PK and restricted-without-PK scenarios.

- `fieldsWithRelational`: the previous `// CRITICAL: Always include the
  primary key field` comment was misleading because sanitize permission-
  filters the PK and language-code path immediately after. Comment now
  states the truth: PK + language code are added before the gate so
  sanitize can drop them when the user lacks read access, matching
  native Directus's permission-filtered `useCollection.primaryKeyField`
  (degraded interaction instead of an empty 403).
- Drop the unused `probing` ref from `useTranslationLanguages` (declared,
  set, never consumed).
- Strip issue/bug-ID references from in-source comments per CLAUDE.md —
  ticket pointers rot as the codebase evolves; PR descriptions own that
  history.
- Compress over-long JSDoc and inline blocks in `sanitizeFilter`,
  `resolveTranslationValue`, `useTranslationLanguages`, and several
  spots in `super-table.vue`. WHY preserved, WHAT removed.
- Add two regression tests documenting `sanitizeFields`' contract:
  primary key and `translations.languages_code` are both dropped when
  not in the user's read whitelist — callers rely on this to fall back
  gracefully.

Net: +84 / -109 (-25 lines), 8 files.

Author: smartlabsAT

CHANGELOG.md

@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `combinedFilter` no longer mixes side-effects with computed evaluation; the user notification is now emitted from a dedicated watcher.
 - `PermissionAction` union no longer includes the unused `'share'` action.
 - `usePermissions` guards against array-shaped permission stores (legacy / future Directus shape changes) instead of silently failing.
+- `fieldsWithRelational` clarifies its behaviour: the primary key and translation language code path are added BEFORE the permission gate, and sanitize drops them only if the user lacks read permission. This mirrors native Directus's `useCollection.primaryKeyField`, which is itself permission-filtered, so users without PK read access see the same graceful degradation in both layouts (items render with limited interaction) instead of an empty 403 error state.
 
 ### Known limitations
 - Bulk-action **Edit / Delete / Add Item** buttons (rendered by Directus Core, not by this extension) remain visible-but-disabled when the user lacks the corresponding permission. A future Directus core PR is required to fully hide them.

src/components/EditableCellRelational.vue

@@ -257,9 +257,9 @@ const displayValue = computed(() => {
     return props.edits;
   }
 
-  // Special handling for translations fields (issue #37 bug C)
-  // Delegated to a centralized helper to guard against translation-row objects
-  // leaking through and rendering as the literal "[object Object]".
+  // Translation sub-fields go through the centralised helper so a sibling
+  // re-render during a popover open cannot leak a whole translation row
+  // through as "[object Object]".
   if (actualFieldKey.value.includes('translations.')) {
     return resolveTranslationValue(
       props.item,
@@ -269,10 +269,8 @@ const displayValue = computed(() => {
     );
   }
 
-  // Handle relational fields with display templates.
-  // Resolved priority: override → field-display → heuristic → none.
-  // Issue #48: layout-level override (columnDisplays) takes priority over the
-  // field-settings display.
+  // Display-template resolution priority: column-display override →
+  // field's own display template → relational heuristic → none.
   const storageKey = props.fieldKey.includes(':') ? props.fieldKey.split(':')[0] : props.fieldKey;
   const override = props.columnDisplays?.[storageKey];
   const fieldTemplate =

src/components/InlineEditPopover.vue

@@ -691,9 +691,9 @@ function handleCellClick(toggle: Function) {
 function formatDisplayValue(value: any): string {
   if (value === null || value === undefined) return '—';
 
-  // Defensive guard (issue #37 bug C): a translation-row object leaked through
-  // as the cell value (e.g. when sibling cells re-render while another cell's
-  // popover opens). Extract its `text` so we never render "[object Object]".
+  // Defensive guard: a translation-row object can leak through as the cell
+  // value (e.g. when a sibling re-renders while another cell's popover opens).
+  // Extract `text` so we never render "[object Object]".
   if (
     value &&
     typeof value === 'object' &&

src/composables/useTranslationLanguages.ts

@@ -2,28 +2,19 @@ import { onMounted, ref, watch, type Ref } from 'vue';
 import { useApi } from '@directus/extensions-sdk';
 
 /**
- * Probes the user's accessible languages by querying which `languages_code`
- * values appear in the translations junction collection. The server enforces
- * the row-level filter automatically, so the result reflects exactly the
- * languages the user can read — including row-level restrictions that the
- * client cannot otherwise inspect (Directus does not expose raw permission
- * filters via /permissions/me).
- *
- * Returns:
- *   - non-empty `string[]` when the probe succeeded and the collection had
- *     at least one accessible row,
- *   - `null` when the probe failed (network error, 403, etc.) — caller is
- *     expected to fall back to permission-based detection,
- *   - `null` when the collection is empty (no rows exist at all) so callers
- *     don't accidentally drop every language column on a fresh installation.
+ * Probe the translations junction for the language codes the user can actually
+ * read. The server applies row-level permission filters during the aggregate,
+ * so the result captures restrictions Directus does not expose via
+ * `/permissions/me`. Returns `null` on failure or when the collection has no
+ * accessible rows — the caller is expected to fall back to a permission-store
+ * lookup so an empty collection doesn't silently drop every language column.
  */
 export function useTranslationLanguages(
   translationsCollection: Ref<string | null>,
   languageCodeField: Ref<string>
 ) {
   const api = useApi();
   const probedLanguages = ref<string[] | null>(null);
-  const probing = ref(false);
 
   async function probe(): Promise<void> {
     const collection = translationsCollection.value;
@@ -32,7 +23,6 @@ export function useTranslationLanguages(
       probedLanguages.value = null;
       return;
     }
-    probing.value = true;
     try {
       const response = await api.get(`/items/${collection}`, {
         params: {
@@ -48,15 +38,12 @@ export function useTranslationLanguages(
       probedLanguages.value = codes.length > 0 ? codes : null;
     } catch {
       probedLanguages.value = null;
-    } finally {
-      probing.value = false;
     }
   }
 
-  // Re-probe on subsequent ref changes. Initial probe is deferred to onMounted
-  // so we don't force evaluation of upstream computeds (e.g. hasTranslationFields)
-  // before they are initialized in the parent setup() — a previous attempt with
-  // `{ immediate: true }` triggered a TDZ error.
+  // The initial probe is deferred to onMounted so upstream computeds (e.g.
+  // `hasTranslationFields`) are initialized before the watcher's reactive
+  // tracking touches them — `{ immediate: true }` here triggers a TDZ error.
   watch([translationsCollection, languageCodeField], () => {
     if (translationsCollection.value) probe();
     else probedLanguages.value = null;
@@ -66,5 +53,5 @@ export function useTranslationLanguages(
     if (translationsCollection.value) probe();
   });
 
-  return { probedLanguages, probing, probe };
+  return { probedLanguages, probe };
 }

src/super-table.vue

@@ -388,12 +388,10 @@ const perPageOptions = PER_PAGE_OPTIONS;
 const permissions = usePermissions();
 const filterSanitizationNotified = ref(false);
 
-// Language items for v-select.
-// Uses the permission-store list (all languages the user can read in principle)
-// rather than the probe-based effectiveAccessibleLanguages — when adding columns
-// we want to offer every language the user could ever see, not just ones with
-// data right now. Column rendering itself still uses the stricter probe-based
-// list for tableHeaders/sanitizeFields.
+// "Add column" picker uses the permission-store list (every language the user
+// could ever see) rather than the probe-based `effectiveAccessibleLanguages`,
+// so empty languages still appear as options. Header rendering keeps the
+// stricter probe list.
 const languageItems = computed(() => {
   const baseList =
     languages.value && languages.value.length > 0 ? languages.value : DEFAULT_LANGUAGES;
@@ -449,8 +447,8 @@ const sortAllowed = computed(() => {
 // Use pagination composable
 const { page, limit } = useTablePagination(layoutQuery as any);
 
-// Use sort composable — pass collection + fieldsStore so stale sort fields
-// referencing deleted columns are silently dropped (issue #47).
+// Pass collection + fieldsStore so stale sort fields referencing deleted
+// columns are silently dropped instead of triggering 400s on the next fetch.
 const { sort, tableSort, onSortChange } = useTableSort(
   layoutQuery as any,
   collection as Ref<string | null>,
@@ -508,32 +506,24 @@ const { aliasedFields, aliasQuery, getFromAliasedItem } = useAliasFields(
   columnDisplaysRef
 );
 
-// Translation collection lookup (parent → junction). Used both for the
-// permission gate and for the row-level language probe below.
-// Declared AFTER `fields` is initialized because `hasTranslationFields`
-// reads `fields.value` and Vue's watcher in useTranslationLanguages tracks
-// dependencies eagerly during setup.
+// Must be declared after `fields` / `hasTranslationFields` so the watcher
+// inside `useTranslationLanguages` doesn't touch them before initialisation
+// (Vue tracks reactive dependencies eagerly during setup → TDZ otherwise).
 const translationsCollectionRef = computed<string | null>(() => {
   if (!hasTranslationFields.value) return null;
   return (
     relationsStore.getRelationsForField(collection.value, 'translations')?.[0]?.collection ?? null
   );
 });
 
-// Probe which languages the user can actually read from translations rows.
-// `languages` collection permission is just a proxy — when admins restrict
-// translations row-by-row (e.g. via `languages_code._in: [de-DE, en-GB]`),
-// the probe is the only way to know exact accessible languages.
+// Probe wins over the static permission-store lookup because Directus does
+// not expose row-level filters (e.g. `languages_code._in: [de-DE, en-GB]`)
+// via /permissions/me — the aggregate query is the only way to discover them.
 const { probedLanguages } = useTranslationLanguages(
   translationsCollectionRef,
   computed(() => translationConfig.value.languageCodeField)
 );
 
-// Effective list of accessible languages.
-//   1. Probe result, when available — most accurate (covers row-level filter).
-//   2. Permission-store `getAccessibleLanguages` fallback when probe returned
-//      null (collection empty, probe failed, no translations).
-//   3. Empty array when neither source has data (defense in depth: no filter).
 const effectiveAccessibleLanguages = computed<string[]>(() => {
   if (probedLanguages.value && probedLanguages.value.length > 0) {
     return probedLanguages.value;
@@ -550,22 +540,21 @@ const fieldsWithRelational = computed(() => {
     return aliasInfo.fields || [aliasInfo.key];
   });
 
-  // Remove duplicates
   const adjustedFields = [...new Set(allDisplayFields)];
 
-  // CRITICAL: Always include the primary key field for navigation and identification
+  // PK + language-code path are added BEFORE the permission gate so sanitize
+  // can drop them when the user lacks read access — matches native Directus,
+  // where `useCollection.primaryKeyField` is permission-filtered and a denied
+  // PK simply degrades interaction (no item-key, no inline edit) rather than
+  // 403'ing the entire fetch.
   const pkField = getPrimaryKeyFieldName();
-  if (!adjustedFields.includes(pkField)) {
-    adjustedFields.unshift(pkField); // Add at the beginning
-  }
+  if (!adjustedFields.includes(pkField)) adjustedFields.unshift(pkField);
 
-  // Ensure language code field is included for translations
-  const languageFieldPath = getTranslationLanguageFieldPath(translationConfig.value);
-  if (hasTranslationFields.value && !adjustedFields.includes(languageFieldPath)) {
-    adjustedFields.push(languageFieldPath);
+  if (hasTranslationFields.value) {
+    const languageFieldPath = getTranslationLanguageFieldPath(translationConfig.value);
+    if (!adjustedFields.includes(languageFieldPath)) adjustedFields.push(languageFieldPath);
   }
 
-  // Permission gate: drop fields/language-suffixed entries the user cannot read
   const translationsCollection = relationsStore.getRelationsForField(
     props.collection,
     'translations'
@@ -921,13 +910,11 @@ const sanitizedFilterResult = computed(() => {
     filters.length === 0 ? undefined : filters.length === 1 ? filters[0] : { _and: filters };
   if (!merged) return { sanitized: undefined, removed: [] as string[] };
 
-  // Permission check is scope-aware: when the walker descends into the
-  // translations junction (via `_some`/`_none`/`_every`), `scope` will be
-  // the junction collection name and we check sub-field permissions there
-  // instead of on the parent collection. Without this, a filter like
-  // `{ translations: { _some: { description: ... } } }` would slip past
-  // the parent `canRead('translations')` check and hit the server with a
-  // sub-field the user can't read — re-opening Bug E for nested filters.
+  // `nestedScopes` makes the walker check sub-fields under `_some`/`_none`/
+  // `_every` against the junction collection rather than the parent —
+  // without it, `{ translations: { _some: { description: ... } } }` would
+  // slip past the parent's `canRead('translations')` check and reach the
+  // server with a sub-field the user cannot read.
   const nestedScopes: Record<string, string | undefined> = {
     translations: translationsCollectionRef.value ?? undefined,
   };

src/utils/resolveTranslationValue.ts

@@ -1,20 +1,9 @@
 /**
- * Safely resolve a translation field value from an item's translations array.
- *
- * Issue #37 (Bug C): When the popover opens for one cell, sibling translation
- * cells could re-render with the full translation row object as their value,
- * which then renders as the literal string "[object Object]" through Vue's
- * default `String()` coercion.
- *
- * This helper centralizes the lookup and guards against the edge case where
- * the resolved sub-field value is itself an object that contains a `text`
- * property (e.g. accidentally double-nested translation rows).
- *
- * @param item - Parent item carrying the `translations` array
- * @param fieldPath - Field key including the leading `translations.` segment (e.g. `translations.text`)
- * @param language - Target language code (e.g. `en-GB`); when null, no lookup is attempted
- * @param languageCodeField - Field on each translation row holding the language code
- * @returns The primitive translation value, or null when no translation matches
+ * Resolve a single translation field value (e.g. `translations.text` for a
+ * given language). Centralised so a sibling cell re-rendering during a popover
+ * open cannot leak the whole translation row through Vue's `String()`
+ * coercion as `"[object Object]"` — the final guard unwraps the row when it
+ * accidentally arrives in place of the sub-field value.
  */
 export function resolveTranslationValue(
   item: any,

src/utils/sanitizeFilter.ts

@@ -7,15 +7,10 @@ export interface SanitizeResult {
 
 export interface SanitizeFilterOptions {
   /**
-   * Map from a parent-field name to the collection it nests into.
-   * When the walker traverses into the value of a field listed here,
-   * inner field names will be checked against the nested collection's
-   * permissions instead of the outer scope.
-   *
-   * Example: `{ translations: 'pages_translations' }`
-   *  - Top-level `translations` is checked against the parent collection.
-   *  - Sub-fields inside relation match operators (`_some`, `_none`, `_every`)
-   *    are checked against `pages_translations`.
+   * Maps a parent-field name (e.g. `translations`) to the collection its
+   * sub-fields live in (e.g. `pages_translations`). When the walker descends
+   * into a `_some`/`_none`/`_every` value under that field, sub-fields are
+   * checked against the nested collection instead of the outer scope.
    */
   nestedScopes?: Record<string, string | undefined>;
 }
@@ -24,18 +19,13 @@ const LOGICAL_KEYS = new Set(['_and', '_or']);
 const RELATION_MATCH_OPS = new Set(['_some', '_none', '_every']);
 
 /**
- * Walk a Directus filter tree and remove conditions that reference fields
- * the caller can't read. Logical operators (`_and`, `_or`) collapse if all
- * their branches are dropped. Relation match operators (`_some`, `_none`,
- * `_every`) recurse into a nested scope when configured via `nestedScopes`,
- * so e.g. `{ translations: { _some: { description: ... } } }` is checked
- * both at the parent level (translations field readable?) and at the
- * junction collection level (description field readable?).
- *
- * The `canRead` callback receives `(field, scope?)`. When `scope` is
- * undefined, the caller should check against the outer/parent collection.
- * When `scope` is set (matches a value from `nestedScopes`), the caller
- * should check against that collection.
+ * Walk a Directus filter tree and remove conditions on fields the caller
+ * cannot read. `_and`/`_or` branches collapse when emptied. Relation match
+ * operators (`_some`/`_none`/`_every`) descend into the configured nested
+ * scope so e.g. `{ translations: { _some: { description: ... } } }` is
+ * checked at both the parent level (`translations` readable?) and the
+ * junction-collection level (`description` readable?). The `canRead`
+ * callback receives the current `scope` (undefined = parent collection).
  */
 export function sanitizeFilter(
   filter: FilterValue,

tests/unit/composables/usePermissions.test.ts

@@ -184,4 +184,27 @@ describe('usePermissions.sanitizeFields', () => {
       })
     ).toEqual(['translations.text:de-DE']);
   });
+
+  // Documents the contract: sanitizeFields treats every field the same way,
+  // including structural fields like the primary key and translation language
+  // code. The `fieldsWithRelational` caller in super-table.vue relies on this
+  // to fall back gracefully — when the user lacks read permission on the PK,
+  // sanitize drops it, the API request omits it, and the layout degrades the
+  // same way native Directus does (items render with limited interaction
+  // instead of an empty 403 error state).
+  it('drops the primary key when it is not in the read whitelist (graceful degradation)', () => {
+    mockPermissions.value.issue_37_test.read.fields = ['title'];
+    const { sanitizeFields } = usePermissions();
+    expect(sanitizeFields('issue_37_test', ['id', 'title'])).toEqual(['title']);
+  });
+
+  it('drops translations.languages_code when sub-field is not in the junction whitelist (graceful degradation)', () => {
+    mockPermissions.value.issue_37_test_translations.read.fields = ['id', 'text'];
+    const { sanitizeFields } = usePermissions();
+    expect(
+      sanitizeFields('issue_37_test', ['translations.languages_code', 'translations.text'], {
+        translationsCollection: 'issue_37_test_translations',
+      })
+    ).toEqual(['translations.text']);
+  });
 });