698 - Audio not showing to Editors#747
Conversation
✅ Deploy Preview for dailp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hello! Quick intro, I'm Sarah, and I'm getting acquainted with the DAILP codebase and onboarding via code reviews. Wanted to give you a heads up that I'll be taking a look at this one; I'll mostly be focused on asking questions and learning for now. |
sarahkelley42
left a comment
sarahkelley42
left a comment
There was a problem hiding this comment.
Mostly asking questions at this moment as I'm growing my understanding of the codebase.
| contributor={ | ||
| audio.recordedBy?.id === userId | ||
| ? "you" | ||
| : audio.recordedBy?.displayName |
There was a problem hiding this comment.
If this evaluates to undefined or null, does AudioPlayer render correctly?
There was a problem hiding this comment.
Right now if undefined is passed into AudioPlayer for contributor, the audio player still renders but it does not say who recorded the audio.
| (audio, index) => | ||
| audio.recordedBy?.id === userId && ( | ||
| {role === UserRole.Contributor && | ||
| (() => { |
There was a problem hiding this comment.
Making sure I understand the structure correctly. Before, this would render:
- To non-Editors (presumably, Readers and Contributors): All edited audio (from
p.doc.editedAudio) as<AudioPlayer>s - To Contributors: All contributed audio (from
p.doc.userContributedAudio, filtered by user ID) as<AudioPlayer>s in a subsequent section - To Editors: All audio (from
p.doc.userContributedAudio, unfiltered) as<DocumentAudioWithCurate>.
After, this will render:
- To Contributors: all contributed audio (from
p.doc.userContributedAudio, filtered by userId), plus all edited audio (fromp.doc.editedAudio), deduped bysliceId, as an<AudioPlayer> - To Readers: all
p.doc.editedAudioas<AudioPlayer>s - To Editors: All
p.doc.userContributedAudioplusp.doc.editedAudio, deduped bysliceId, rendered as<DocumentAudioWithCurate>
So the main behavioral difference here is that Editors now see editedAudio in addition to userContributedAudio. Readers' view is unchanged, and Contributors might see a different arrangement but will see the same set of audio as they saw before.
Is that understanding correct, and does it match the intent?
There was a problem hiding this comment.
Before
- Readers -> One section "Document Audio", rendering
p.doc.editedAudiowith the official audio + any approved audios. Just the AudioPlayer + download button, no curate checkboxes. - Contributors -> The same "Document Audio" section as readers, as well as a second section "User-contributed Audio" which shows only their own uploads, with no curate checkbox. Also the "Record Audio" panel
- Editors -> Only the "User-contributed Audio" section, showing all user contributed audio with curate checkboxes for all of them. Also the "Record Audio" panel. No "Document Audio" section, so editors could not see the official audio
After
"User-contributed Audio" as a section was removed. Every audio now lives in "Document Audio"
- Readers -> All published audio with no checkboxes and no recording section.
- Contributors -> Same as Reader, but also your own uploads and the recording section.
- Editors -> Shows all audio with curate checkboxes for all of them, and the recording section.
I worked with Cara to finalise what exactly we want it to look like and it seems like this is the intended result.
There was a problem hiding this comment.
Cool cool. I think there may be opportunity to express these intents by extracting out a couple functions for the filters, and maybe have something mapping role -> filter, but I'm not totally sure what that would look like at this point. Seems like this does the intended thing.
| index: 0, | ||
| // TODO: is this a bad default? | ||
| include_in_edited_collection: true, | ||
| include_in_edited_collection: item.include_audio_in_edited_collection, |
There was a problem hiding this comment.
I'm assuming this is only impacting reads. Is the value for include_audio_in_edited_collection already set for everything in the database, or does this require migration/data entry?
There was a problem hiding this comment.
It defaults to true.
| { | ||
| "db_name": "PostgreSQL", | ||
| "query": "select\n d.id,\n d.short_name,\n d.title,\n d.is_reference,\n d.written_at,\n d.audio_slice_id,\n media_resource.url as \"audio_url?\",\n media_resource.recorded_at as \"recorded_at?\",\n dailp_user.id as \"recorded_by?\",\n dailp_user.display_name as \"recorded_by_name?\",\n media_slice.time_range as \"audio_slice?\",\n ubd.bookmarked_on as \"bookmarked_on?\",\n coalesce(\n jsonb_agg(\n jsonb_build_object(\n 'name', contributor.full_name, 'role', attr.contribution_role\n )\n ) filter (where contributor is not null),\n '[]'\n )\n as contributors\nfrom document as d\n left join contributor_attribution as attr on attr.document_id = d.id\n left join contributor on contributor.id = attr.contributor_id\n left join media_slice on media_slice.id = d.audio_slice_id\n left join media_resource on media_resource.id = media_slice.resource_id\n left join dailp_user on dailp_user.id = media_resource.recorded_by\n left join user_bookmarked_document as ubd on ubd.document_id = d.id\nwhere d.short_name = any($1)\ngroup by d.id,\n media_slice.id,\n media_resource.id,\n dailp_user.id,\n ubd.bookmarked_on\n", | ||
| "query": "select\n d.id,\n d.short_name,\n d.title,\n d.is_reference,\n d.written_at,\n d.audio_slice_id,\n d.include_audio_in_edited_collection,\n media_resource.url as \"audio_url?\",\n media_resource.recorded_at as \"recorded_at?\",\n dailp_user.id as \"recorded_by?\",\n dailp_user.display_name as \"recorded_by_name?\",\n media_slice.time_range as \"audio_slice?\",\n ubd.bookmarked_on as \"bookmarked_on?\",\n coalesce(\n jsonb_agg(\n jsonb_build_object(\n 'name', contributor.full_name, 'role', attr.contribution_role\n )\n ) filter (where contributor is not null),\n '[]'\n )\n as contributors\nfrom document as d\n left join contributor_attribution as attr on attr.document_id = d.id\n left join contributor on contributor.id = attr.contributor_id\n left join media_slice on media_slice.id = d.audio_slice_id\n left join media_resource on media_resource.id = media_slice.resource_id\n left join dailp_user on dailp_user.id = media_resource.recorded_by\n left join user_bookmarked_document as ubd on ubd.document_id = d.id\nwhere d.short_name = any($1)\ngroup by d.id,\n media_slice.id,\n media_resource.id,\n dailp_user.id,\n ubd.bookmarked_on\n", |
There was a problem hiding this comment.
Confirming my understanding: this JSON (query+the columns description below) looks like it's generated from the .sql files under types/queries. Is that correct?
There was a problem hiding this comment.
They're generated from the SQL queries that are embedded in the rust code. In that file you'll see multiple usages of query!, query_as!, query_file! etc that point to files in the types/queries directory. From my understanding, these files are a cache used for compile-time checking when the database is offline and unreachable.
2 participants
Merged "Document Audio" and "User-contributed audio" into one list.
Editors can see:
Contributors can see:
Readers can see: