Conversation
There was a problem hiding this comment.
Pull request overview
Adds publisher (organisation) information to data sources so the UI can display “Published by ”, particularly for the Movement Data Library and superadmin data source management.
Changes:
- Extend
dataSourcetRPC outputs to includeorganisationNameacross relevant list/detail endpoints. - Display organisation/publisher metadata in
DataSourceItemcards and the inspector’sDataRecordsPanel. - Show organisation name in superadmin data source tables and detail/preview views.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/trpc/routers/dataSource.ts | Enriches data source queries/responses with organisationName; generalises addImportInfo typing. |
| src/components/DataSourceItem.tsx | Adds OrganisationMeta and renders publisher info in multiple density variants. |
| src/app/(private)/map/[id]/components/InspectorPanel/DataRecordsPanel.tsx | Shows publisher info at the bottom of the inspector panel. |
| src/app/(private)/hooks/useDataSourceListCache.ts | Updates cache updater wrapper to include organisationName (but currently risks wiping it). |
| src/app/(private)/(dashboards)/superadmin/page.tsx | Adds an “Organisation” column to the public data sources table. |
| src/app/(private)/(dashboards)/superadmin/data-sources/[id]/page.tsx | Displays organisation name on the superadmin data source detail page. |
| src/app/(private)/(dashboards)/superadmin/data-sources/[id]/components/DefaultInspectorPreview.tsx | Displays organisation name in the preview footer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .selectFrom("dataSource") | ||
| .leftJoin("organisation", "dataSource.organisationId", "organisation.id") | ||
| .where("dataSource.public", "=", true) | ||
| .selectAll("dataSource") | ||
| .select(["organisation.name as organisationName"]) | ||
| .execute(); | ||
|
|
||
| const withImportInfo = await addImportInfo(dataSources); |
There was a problem hiding this comment.
listPublic no longer calls findPublicDataSources(), so the findPublicDataSources import at the top of this file appears to be unused and may fail lint/TS builds with unused-import checks. Remove the unused import or switch back to using the repository helper.
| (old: DataSourceByOrganisation[] | undefined) => | ||
| old?.map((ds) => | ||
| ds.id === dataSourceId | ||
| ? { ...ds, ...updater({ ...ds, organisationOverride: null }) } | ||
| ? { | ||
| ...ds, | ||
| ...updater({ | ||
| ...ds, | ||
| organisationName: "", | ||
| organisationOverride: null, | ||
| }), | ||
| } | ||
| : ds, |
There was a problem hiding this comment.
In updateDataSource, the wrapper passed to updater() overwrites organisationName with an empty string before calling the updater. If the updater returns { ...ds, ... } (common pattern), this will clear an existing organisation name in the cache until the next refetch. Preserve the existing organisationName when present (only default it when missing).
| { queryKey: trpc.dataSource.byId.queryKey() }, | ||
| (old: DataSourceById | undefined) => { | ||
| if (!old || old.id !== dataSourceId) return old; | ||
| return { ...old, ...updater({ ...old, organisationOverride: null }) }; | ||
| return { | ||
| ...old, | ||
| ...updater({ | ||
| ...old, | ||
| organisationName: "", | ||
| organisationOverride: null, | ||
| }), | ||
| }; |
There was a problem hiding this comment.
Same issue for the byId cache: organisationName is set to "" before calling updater(), which can wipe the cached organisation name if the updater spreads its input. Pass through the existing value (or default only when absent) instead of always forcing an empty string.
| "use client"; | ||
|
|
||
| import { Settings } from "lucide-react"; | ||
| import { BookOpen, Settings } from "lucide-react"; |
There was a problem hiding this comment.
BookOpenIcon is imported but never used in this component. Remove the unused import to avoid lint/TS unused-import errors.
| import { BookOpen, Settings } from "lucide-react"; | |
| import { Settings } from "lucide-react"; |
| function OrganisationMeta({ | ||
| organisationName, | ||
| compact, | ||
| }: { | ||
| organisationName: string | null | undefined; | ||
| compact?: boolean; | ||
| }) { | ||
| const name = organisationName?.trim(); | ||
| if (!name) return null; | ||
| return ( | ||
| <span | ||
| className={cn( | ||
| "inline-flex items-center gap-1 whitespace-nowrap text-muted-foreground", | ||
| compact ? "text-[11px]" : "text-xs", | ||
| )} | ||
| > | ||
| Published by | ||
| <span className="text-neutral-400">•</span> | ||
| <span className="truncate">{name}</span> | ||
| </span> | ||
| ); | ||
| } |
There was a problem hiding this comment.
OrganisationMeta always renders the text "Published by" whenever organisationName is set, but it isn’t gated on dataSource.public. This will label private/internal data sources as "Published" in contexts like the organisation’s own data source list where the "Published" badge is not shown. Consider rendering this meta only when dataSource.public is true, or change the wording to something that doesn’t imply the public flag.
| <div className="flex items-center gap-1 pt-2 border-t border-neutral-200/70 text-muted-foreground"> | ||
| <BookOpen className="w-3 h-3 shrink-0" /> | ||
| <p className="text-[11px] truncate"> | ||
| Published by <span className="text-neutral-400">•</span>{" "} |
There was a problem hiding this comment.
This UI shows "Published by" based solely on dataSource.organisationName. If dataSource.public is false (private/internal data sources in the inspector), this copy becomes misleading compared to the existing "Published" badge semantics elsewhere. Consider gating this on dataSource.public or adjusting the copy to not imply publication.
| Published by <span className="text-neutral-400">•</span>{" "} | |
| {dataSource.public ? "Published by" : "Provided by"}{" "} | |
| <span className="text-neutral-400">•</span>{" "} |
Description
Motivation and Context
How Can It Be Tested?
How Will This Be Deployed?
Screenshots (if appropriate):
Types of changes
Checklist: