Skip to content

Translate: show where linked strings live when active filters don't match#4281

Open
ayshushus wants to merge 4 commits into
mozilla:mainfrom
ayshushus:2921
Open

Translate: show where linked strings live when active filters don't match#4281
ayshushus wants to merge 4 commits into
mozilla:mainfrom
ayshushus:2921

Conversation

@ayshushus

@ayshushus ayshushus commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Fix #2921

The string URL param permalinks a specific string, but shared links also carry the filters active at link time (status=missing, search, ...). When the target doesn't match those filters (e.g. link says status=missing but the string is now translated), we silently redirected to the first list entry + a toast, landing users on an unrelated string. Now surface where the string actually lives and let the user choose.

Backend:

  • base.views._get_paginated_entities: when the requested entity is filtered out, run a second, filter-free lookup gated only by viewability (obsolete=False, translatedresources__locale, project not disabled/system, Project.objects.visible_for(user)) and attach requested_entity_location ({pk, project slug, resource path}) to the response. Absent when the string is invalid/obsolete/not viewable, so behavior is unchanged there. Thread user through for the visibility gate.

State:

  • api/entity.ts: add RequestedEntityLocation type, optional requested_entity_location on the response.
  • entities/actions.ts, reducer.ts: carry it onto RECEIVE_ENTITIES, store as requestedEntityLocation. Preserve across pages (backend only reports on page 1), clear on RESET.
  • entities/hooks.ts: useEntityNotFound()show when entity > 0 && requestedEntityLocation != null && !found.

UI:

  • entities/components/StringNotFound.{tsx,css}: new editor-panel screen. "See string" → push({ ...emptyParams, project, resource, entity }) (keep string, drop filters; navigates cross-project). "See other strings" → push({ entity: 0 }) (keep filters, drop string).
  • App.tsx: render <StringNotFound> over <Entity>/<BatchActions> when useEntityNotFound().show.
  • entitieslist/EntitiesList.tsx: skip the "select first entity" redirect + ENTITY_NOT_FOUND toast when the string-not-found screen applies.
  • context/Location.tsx: export emptyParams (consumed by StringNotFound).
  • locale/en-US/translate.ftl: add entities-StringNotFound--* strings.

Tests:

  • test_entities_not_matching_string_reports_location: reports location when filtered out; absent when the string matches.
  • StringNotFound.test.jsx: both navigation paths (incl. cross-project) + null-location renders nothing.

@ayshushus ayshushus self-assigned this Jul 3, 2026
@ayshushus ayshushus requested a review from eemeli July 3, 2026 18:03
@ayshushus ayshushus marked this pull request as ready for review July 3, 2026 18:03
@ayshushus

Copy link
Copy Markdown
Collaborator Author

TODO

We should file follow-up issues to:

Inform users when non-string URL parameters are ignored.
Show the regular 404 page when the resource in the path doesn't exist.

@ayshushus ayshushus requested review from eemeli and functionzz and removed request for eemeli July 7, 2026 13:16

@eemeli eemeli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks decent, though I only did a surface-level review pass here.

Comment thread pontoon/base/views.py
),
}
if requested_entity_location is not None:
response["requested_entity_location"] = requested_entity_location

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

requested_entity_location feels like a pretty verbose name for this in the API, why not just requested_entity?

parts.push('other filters');
}
return parts.join(', ');
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be localized, and the concatenation should use Intl.ListFormat.

Comment on lines +86 to +89
{`String ${string} is in ${stringResource} (${stringProject}). ` +
(filters
? `You're browsing ${queryLabel}, filtered by ${filters}.`
: `You're browsing ${queryLabel}.`)}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should really be able to rely on our own localization system, rather than duplicating the English strings all over the place.

}

const { push } = location;
const string = String(entityLocation.pk);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should probably be called something like stringId.

Comment on lines +25 to +26
show: entity > 0 && requestedEntityLocation != null && !found,
entityLocation: requestedEntityLocation,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why can't we rely only on whether requestedEntityLocation is empty?

Comment on lines +161 to +162
const stringNotFound =
selectedEntity > 0 && !isValid && requestedEntityLocation != null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also here, why not only check requestedEntityLocation? The check right below also uses !selectedEntity, and it's not clear why all of these conditions matter.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it'd be useful for e.g. @flodolo to look over these strings.

## String not found page
## Shown when the requested string doesn't match the rest of the query

entities-StringNotFound--description =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This would never pass review in one of our projects. Variants should be fully formed strings, and variants are not guaranteed to exist in the localized version (they're private). So, this is not a good use of variants.

What's $filters? Can we support multiple strings and selection at run-time instead of variants?

It would be great if you could share a screenshot of how the page looks like.

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.

In query parameters, the string should take priority over others

3 participants