pierrejeambrun commented on code in PR #75229:
URL: https://github.com/apache/airflow/pull/75229#discussion_r4338208456
##########
airflow-core/src/airflow/ui/src/utils/links.ts:
##########
@@ -184,3 +187,130 @@ export const buildTaskInstanceUrl = (params: {
return `${basePath}${additionalPath}`;
};
+
+/**
+ * Where a plugin link should open: in this tab for the UI's own origin, a new
one otherwise.
+ *
+ * A URL that cannot be parsed is treated as external, so a malformed plugin
link never navigates the
+ * app away from the page the user is on.
+ */
+export const getLinkTarget = (url: string): "_blank" | "_self" => {
+ try {
+ return new URL(url, globalThis.location.origin).origin ===
globalThis.location.origin
+ ? "_self"
+ : "_blank";
+ } catch {
+ return "_blank";
+ }
+};
+
+const decodeSegments = (path: string) =>
+ path.split("/").map((segment) => {
+ try {
+ return decodeURIComponent(segment);
+ } catch {
+ // A malformed escape is not decodable; compare it as written.
+ return segment;
+ }
+ });
+
+/**
+ * Whether two paths address the same page, compared segment by segment once
decoded.
+ *
+ * A link action's identifiers arrive percent-encoded from
`applyPluginTemplate`, while the
+ * router's own `location.pathname` carries them raw -- a run id like
+ * `manual__2026-10-20T12:00:00+00:00` is `...T12%3A00%3A00%2B00%3A00` on one
side and keeps its
+ * `:` and `+` on the other. Comparing the strings would call every run-scoped
page a different
+ * page. Segments are decoded individually so an encoded `/` stays a value
rather than becoming
+ * a boundary.
+ */
+const isSamePath = (left: string, right: string): boolean => {
+ const leftSegments = decodeSegments(left);
+ const rightSegments = decodeSegments(right);
+
+ return (
+ leftSegments.length === rightSegments.length &&
+ leftSegments.every((segment, index) => segment === rightSegments[index])
+ );
+};
+
+/**
+ * The query string a link should navigate with, keeping the current one when
it stays put.
+ *
+ * A key the link names replaces every value the page had for it, rather than
adding to them,
+ * so a link to `?state=failed` means failed rather than "failed as well as
whatever was
+ * already selected" -- while a link that repeats a key keeps all of its own
values.
+ */
+const mergeSearch = (
+ pathname: string,
+ linkParams: URLSearchParams,
+ currentLocation?: { pathname: string; search: string },
+): string => {
+ if (currentLocation === undefined || !isSamePath(currentLocation.pathname,
pathname)) {
+ return linkParams.toString() === "" ? "" : `?${linkParams}`;
+ }
+
+ const merged = new URLSearchParams(currentLocation.search);
+
+ for (const key of new Set(linkParams.keys())) {
+ merged.delete(key);
+ }
+ for (const [key, value] of linkParams) {
+ merged.append(key, value);
+ }
+
+ return merged.toString() === "" ? "" : `?${merged}`;
+};
+
+/**
+ * Resolve `url` to a path this app's router owns, or `undefined` if it is not
one.
+ *
+ * Matched against the router's own route table rather than just the base
path, because a
+ * same-origin URL is not necessarily a UI page -- `/api/v2/...` and a
plugin's own FastAPI
+ * pages sit under the same prefix and must be followed as ordinary links. The
table ends in a
+ * catch-all that renders the error page, so landing on that leaf means "no UI
route for this
+ * path", not "internal".
+ *
+ * The result is relative to the base path, ready to hand to a router link,
which adds the
+ * prefix back itself.
+ *
+ * Pass `currentLocation` to keep the query string the user already has when
the link stays on
+ * the same page. A link's target is fixed when the plugin loads, so without
this a button on
+ * the Dags list that only wants to add one parameter would navigate with its
own query string
+ * and silently drop every filter, search term and sort the user had set. Keys
the link names
+ * win; the rest survive.
+ *
+ * Only when the path is unchanged: across pages the parameters belong to the
page being left
+ * -- carrying a Dag Runs `?state=failed` onto Task Instances would filter
something the user
+ * never asked to filter.
+ */
+export const getInternalRouteTo = (
+ url: string,
+ currentLocation?: { pathname: string; search: string },
+): string | undefined => {
+ let parsed: URL;
+
+ try {
+ parsed = new URL(url, globalThis.location.origin);
Review Comment:
To be explicit about the leading-slash case, which I think needs fixing
rather than documenting: on a `/d12345/` deployment, `"/dags/{DAG_ID}/runs"`
opens `/dags/x/runs` in a new tab, which renders a blank page (the router
basename doesn't match). We should never end up on a blank page here. A
root-absolute href that matches a UI route should be resolved under the base
path and navigate in-app to `/d12345/dags/x/runs`, without plugin authors
having to prepend `[api] base_url` themselves.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
