Feat(Popup): Use external data to find categorical labels by arnaudfnr · Pull Request #185 · dataforgoodfr/14_Data4Trees

arnaudfnr · 2026-07-05T20:50:37Z

WHAT CHANGES

Backend

Add endpoint to return the content of a list of catalog resources
Update Forest Inventory external data with latest update from Cathalijne

Frontend

Move Fallback renderer component to its own module so it can be reused, + improve design
Add api client methods to fetch external data
Modify popup rendered to accept Promisefunction for external data
Add ErrorBoundary & Suspense for Popup rendering, because now it needs to load external data (maybe we could make the load fail non blocking)
Use external data to find labels of forest name, tree species & taxons in forest inventory

=> No labels have been provided for socio eco data yet

Capture.video.du.2026-07-05.23-00-51.mp4

david-bretaud-dev · 2026-07-05T21:50:48Z

      try {
        const isValid = await verifyToken(token);
        if (isValid) {
-          setToken(storedToken);


Good catch 🤦

david-bretaud-dev · 2026-07-05T21:54:35Z

+    if not request.user.has_perm("users.view_data"):
+        return Response(status=status.HTTP_403_FORBIDDEN)


We can define a custom class CanViewData follow this practice: https://www.django-rest-framework.org/api-guide/permissions/#setting-the-permission-policy. Permissions classes would be "IsAuthenticated & CanViewData"

I know but I went for the less code / quicker option here 😄

david-bretaud-dev · 2026-07-05T21:55:21Z

+@api_view(["POST"])
+@permission_classes([IsAuthenticated])
+def resource_list_view(request, layer_id):
+    # authorization: ensure user has the custom add_data permission


Suggested change

# authorization: ensure user has the custom add_data permission

# authorization: ensure user has the custom view_data permission

david-bretaud-dev · 2026-07-05T21:56:41Z

+
+    resources_payload = {}
+    for resource_name in resource_list:
+        target_path = catalog_path / f"{layer_id}" / f"{resource_name}.parquet"


Shall we check layer id is defined or an existing path ?

david-bretaud-dev · 2026-07-05T22:02:28Z

            data={data.treeDiversity.relative_abundance}
-            metadata={metadata}
+            externalData={metadata}
+            proj={rawData.projet}


Can we keep full-verbose naming for proops ? "project" instead of "proj" ?

david-bretaud-dev · 2026-07-05T22:20:52Z

+// t must be passed to the fallback render function because the error boundary fallback component cannot call hooks like useTranslation
+// See https://react.dev/warnings/invalid-hook-call-warning
+// > 🔴 Do not call Hooks in class components.
+// while ErrorBoundary is a class component: https://github.com/bvaughn/react-error-boundary/blob/main/lib/components/ErrorBoundary.tsx#L45
+export function getFallbackRender({
+  retry,
+  t,
+}: {
+  retry?: () => void;
+  t: TFunction<"common", undefined>;
+}) {
+  function FallbackRender({ error }: FallbackProps) {


You can just extract the JSX into a Functional component, no ?

const FallbackRender= (...) => { const { t } = useTranslation(); ... } <ErrorBoundary fallbackRender={(...) => <FallbackRender ... />} ``` You can't use hooks in class components but you can use Functional Components though

maybe 🤷 I just reused the code of @severo from #175 and I did not catch that

david-bretaud-dev · 2026-07-05T22:23:01Z

+        />
+      </Suspense>
+    </ErrorBoundary>
+  );


Comments on the behavior:

Now people have to be logged in to see data => Can't share data to the public now, so, it wouldn't make sense to be able to land on the map view

IMO, the boundary should be restricted to the data slice it is related to. Here, it's to fetch the label only, right ? This should be the data slice wrapped in the Boundary.

But then comes again the question about: what is pubicly accessible, what should be hidden, what to dislay in case of failure (for public access) ?

Well your comments are absolutely right.
In the new final data, there is a new field "conf" for confidentiality that I did not integrate yet.

So the behavior will be :

When a user is not connected, they will see all data publicly available (conf=1 , yes that's illogic)

External data won't be protected (to confirm with Cathalijne, so yes the current behavior is going to change) so even public users will see the labels. But this requires to think about the backend a little bit here, that's why I protected the resources for now. We can also open it for the time we think about that.

You are right, maybe we can just wrap the indicators that require the promise inside the boundary. We could also make a silent fail with a fallback to values. I think I just wanted to play with the ErroBoundary stuff after having seen it from @severo ^^

david-bretaud-dev · 2026-07-05T22:26:06Z

+  // Retrieve external data before rendering popup
+  const [reloadKey, setReloadKey] = useState(0);
+  const externalDataPromise = useMemo(
+    () =>
+      fetchData({
+        force: reloadKey > 0,
+        promiseFunc,
+      }),
+    [reloadKey, promiseFunc],
+  );
+
+  const retry = useCallback(() => {
+    setReloadKey((k) => k + 1);
+  }, []);
+  const fallbackRender = useMemo(
+    () => getFallbackRender({ retry, t }),
+    [retry, t],
+  );


As far as we used it in my company, so far, you pass the retry function to the FallbackRender, it contains a reload button, and that's all : on demand refetch from the error boundary. No need for state, memoization, callbacks, cache.

Interally, we can do a retry on fail for 3 times for instance (it's just a loop while getting error. But things can be remain simple

I see. Maybe we could improve that later, it's a bit off of what I'm trying to do here and as I said in the previous command it's basically a copy and paste of @severo work, can we have the debate on mattermost it it's important to you ?

david-bretaud-dev · 2026-07-05T22:27:34Z

+
+  return (
+    <ErrorBoundary
+      fallbackRender={fallbackRender}


The fallback render component should have the same height limitation as provided in childrenProps.className

david-bretaud-dev · 2026-07-05T22:29:57Z

    "undefined": "not found"
  },
  "error": {
+    "pleaseLogon": "Please sign in",


Suggested change

"pleaseLogon": "Please sign in",

"pleaseLogin": "Please sign in",

(french key)

david-bretaud-dev

A couple of improvements we can bring to the code.

The main thing that struck me though was the need to be logged in to see the full popup -not just the label-
(Since we have access to the whole style.json anyway, this limitation does not rely exist)

arnaudfnr linked an issue Jul 5, 2026 that may be closed by this pull request

Chore(Webapp): Use labels in external data instead of form data #177

Open

arnaudfnr added 8 commits July 5, 2026 23:21

Feat(Popup): Use external data to find categorical labels

26fec63

Generalize external data fetching

dc61ada

Add missing files

ad3ac71

Improve Fallback Renderer

3cff431

Fix lint

bf6d9ea

Use external data for taxon & tree labels

8cdc2ee

Clean code

e9b04b0

Remove backend unused import

3a176a2

arnaudfnr force-pushed the arnaudfnr/177-use_external_data_for_labels branch from fccda2b to 3a176a2 Compare July 5, 2026 21:21

arnaudfnr marked this pull request as ready for review July 5, 2026 21:24

david-bretaud-dev reviewed Jul 5, 2026

View reviewed changes

david-bretaud-dev requested changes Jul 5, 2026

View reviewed changes

arnaudfnr added 2 commits July 7, 2026 08:41

Fix review

84ca748

remove unused import

8636534

		if not request.user.has_perm("users.view_data"):
		return Response(status=status.HTTP_403_FORBIDDEN)

	# authorization: ensure user has the custom add_data permission
	# authorization: ensure user has the custom view_data permission

	"pleaseLogon": "Please sign in",
	"pleaseLogin": "Please sign in",

Uh oh!

Conversation

arnaudfnr commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHAT CHANGES

Backend

Frontend

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-bretaud-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arnaudfnr commented Jul 5, 2026 •

edited

Loading