[wip] Import display name from orderly custom metadata, store it in proxy DB, and display it #142
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
=======================================
Coverage 97.25% 97.25%
=======================================
Files 131 131
Lines 1238 1238
Branches 346 346
=======================================
Hits 1204 1204
Misses 33 33
Partials 1 1 ☔ View full report in Codecov by Sentry. |
david-mears-2
changed the title
| private fun getDisplayNameForPacket(packet: OutpackMetadata): String { | ||
| val metadata = outpackServerClient.getMetadataById(packet.id) | ||
| val orderlyMetadata = metadata?.custom?.get("orderly") as? Map<*, *> | ||
| val description = orderlyMetadata?.get("description") as? Map<*, *> | ||
| return description?.get("display") as? String ?: packet.name | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm surprised you need to do this - is the custom metadata not already in the metadata being returned for all new packets in line 59? If it's not it should be. Maybe it is but those values ujust aren't being passed out by getMetadata? We shouldn't need to do a new call to the client per packet.
There was a problem hiding this comment.
It does seem inefficient doesn't it. Line 59 corresponds to calling /packit/metadata on the outpack server, which returns only id, name, parameters, and time, contrary to the documentation of outpack server: mrc-ide/outpack_server#68
There was a problem hiding this comment.
So we'll change the outpack server
There was a problem hiding this comment.
Changed outpack server in mrc-ide/outpack_server#69
| @@ -15,6 +15,7 @@ export const PacketTable = () => { | |||
| const { packetName } = useParams(); | |||
| const [pageNumber, setPageNumber] = useState(0); | |||
|
|
|||
| // Rename useGetPacketGroups to useGetPacketGroupPages, and packetGroups to packetGroupPages? | |||
There was a problem hiding this comment.
When you iterate over those things, the elements aren't packet groups, they are pages of packet groups. So later in the file we have eg 'packetGroups.first' which looks like it means 'the first packet group' but actually means 'the first page of packet groups'.
There was a problem hiding this comment.
Oh, I mistook this file for being PacketGroupSummaryList.tsx. In fact, each item returned by useGetPacketGroups (each so-called packetGroup) is a page of packets, not a packet group nor a page of packet groups. So these things should be renamed to useGetPacketPages and packetPages respectively.
There was a problem hiding this comment.
or more simply, useGetPackets and pages
… =/= display-name
…lso packet page to use long-description.)
|
I wonder if we have any CI checks designed to notice when test coverage is too low -- I know my last commit is changing a lot of things that demand new unit tests! |
In this branch there's a new packet group in demos, since none of the existing ones were making use of the description.display_name orderly property.
That property is referenced during the scheduled job that imports packets into the proxy db, so that the column 'display_name' is being used for the first time. When there's no display name, it defaults to the name.
Then, the home page / packet group index page is now able to display the display name for a packit group, which is found by the API looking up the latest display name at the same time as it looks up the latest ID and latest time.
Future iterations: