Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Add media type to content provider #149

Merged
merged 6 commits into from
Aug 3, 2021
Merged

Add media type to content provider #149

merged 6 commits into from
Aug 3, 2021

Conversation

dhruvkb
Copy link
Member

@dhruvkb dhruvkb commented Jul 21, 2021

Since we are now adding new media types, the ContentProvider model needs to specify which media type it provides. This PR adds the new media_type field to the ContentProvider model.

@dhruvkb dhruvkb marked this pull request as ready for review July 21, 2021 10:00
@dhruvkb dhruvkb requested a review from a team as a code owner July 21, 2021 10:00
@dhruvkb dhruvkb requested review from krysal and obulat July 21, 2021 10:00
Copy link
Contributor

@obulat obulat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about providers with several media types, like Wikimedia or Europeana? Is an array a good option for media_type field?

@dhruvkb
Copy link
Member Author

dhruvkb commented Jul 21, 2021

It'll make filtering complicated. Can we not keep two different providers with the same display name but different IDs (like Wikimedia internally stored as wikimedia_images and wikimedia_audio)?

@dhruvkb dhruvkb requested a review from obulat August 2, 2021 08:50
Copy link
Contributor

@obulat obulat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@zackkrida
Copy link
Member

two different providers with the same display name but different IDs

Yeah i think this is fine for now.

@dhruvkb dhruvkb merged commit c27aeac into main Aug 3, 2021
@dhruvkb dhruvkb deleted the provider_model branch August 3, 2021 18:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants