Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_bnf_entry_from_isbn: add edition contributors #673

Merged
merged 1 commit into from Oct 20, 2023

Conversation

maxlath
Copy link
Member

@maxlath maxlath commented Mar 27, 2023

Namely: translators, preface and postface authors

@maxlath maxlath requested a review from jum-s March 27, 2023 14:22
@@ -6,7 +6,7 @@ import { forceArray } from '#lib/utils/base'
import getInvEntityCanonicalUri from '../get_inv_entity_canonical_uri.js'
import properties from '../properties/properties_values_constraints.js'

export default async (claims, resolveOnWikidata = true) => {
export async function resolveExternalIds (claims, resolveOnWikidata = true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ah thx for that, much more greppable with named function

export async function setEditionContributors (edition, property, urls) {
const uris = await resolveContributors(urls)
if (uris.length > 0) {
edition.claims[property] = uris
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a common pattern of seeds sometimes assigning translators as P50. But this will not conflict with the possibility of already having those authors in the P50 claim, right? aka BNF based entity creation is not based on the seed, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

That should not conflict, as far as I can tell

@jum-s
Copy link
Contributor

jum-s commented Sep 25, 2023

wanted to rebase and merge, but tests need a functionnal BNF API which is not the case at the moment

@jum-s jum-s self-requested a review September 25, 2023 04:14
@maxlath maxlath force-pushed the add-edition-contributors-to-bnf-seeds branch from 47bdd22 to 9b7a98e Compare October 20, 2023 17:25
@maxlath maxlath force-pushed the add-edition-contributors-to-bnf-seeds branch from 9b7a98e to 80cf9ae Compare October 20, 2023 17:27
@maxlath maxlath merged commit 3ef70b6 into master Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants