From 68dc48945c983a4b462d1e4e984c36749675ac86 Mon Sep 17 00:00:00 2001 From: David Scotson Date: Fri, 29 Sep 2023 16:06:11 +0100 Subject: [PATCH] feature add ids to works fixes #3430 and #1797 Not fully ready to merge, for at least two reasons: Displays the ids on the work/edition page though #3430 currently suggests this is optional, so may need removed until a design is settled. --- openlibrary/plugins/openlibrary/js/edit.js | 43 ++++++++++++ openlibrary/plugins/openlibrary/js/index.js | 4 ++ openlibrary/plugins/upstream/addbook.py | 3 + openlibrary/plugins/upstream/models.py | 63 +++++++++++++++++- openlibrary/plugins/upstream/utils.py | 21 ++++++ openlibrary/templates/books/edit.html | 69 ++++++++++++++++++++ openlibrary/templates/type/edition/view.html | 14 ++++ 7 files changed, 216 insertions(+), 1 deletion(-) diff --git a/openlibrary/plugins/openlibrary/js/edit.js b/openlibrary/plugins/openlibrary/js/edit.js index 5c216e84bc8f..62efd0da1c09 100644 --- a/openlibrary/plugins/openlibrary/js/edit.js +++ b/openlibrary/plugins/openlibrary/js/edit.js @@ -230,6 +230,13 @@ export function initIdentifierValidation() { }); } +export function initWorkIdentifierValidation() { + $('#workidentifiers').repeat({ + vars: {prefix: 'work--'}, + validate: function(data) {return validateWorkIdentifiers(data)}, + }); +} + export function initClassificationValidation() { const dataConfig = JSON.parse(document.querySelector('#classifications').dataset.config); $('#classifications').repeat({ @@ -248,6 +255,42 @@ export function initClassificationValidation() { }); } +/** + * Called by initWorkIdentifierValidation(), along with tests in + * tests/unit/js/editEditionsPage.test.js, to validate the addition of new + * identifiers (ISBN, LCCN) to an edition. + * @param {Object} data data from the input form + * @returns {boolean} true if identifier passes validation + */ +export function validateWorkIdentifiers(data) { + const dataConfig = JSON.parse(document.querySelector('#workidentifiers').dataset.config); + + if (data.name === '' || data.name === '---') { + return error('#workid-errors', 'workselect-id', dataConfig['Please select an identifier.']) + } + const label = $('#workselect-id').find(`option[value='${data.name}']`).html(); + if (data.value === '') { + return error('#workid-errors', 'workid-value', dataConfig['You need to give a value to ID.'].replace(/ID/, label)); + } + + let validId = true; + if (data.name === 'lccn') { + validId = validateLccn(data, dataConfig, label); + } + + // checking for duplicate identifier entry on all identifier types + // expects parsed ids so placed after validate + const entries = document.querySelectorAll(`.${data.name}`); + if (isIdDupe(entries, data.value) === true) { + return error('#workid-errors', 'workid-value', dataConfig['That ID already exists for this work.'].replace(/ID/, label)); + } + + if (validId === false) return false; + + $('#workid-errors').hide(); + return true; +} + export function initLanguageMultiInputAutocomplete() { $(function() { getJqueryElements('.multi-input-autocomplete--language').forEach(jqueryElement => { diff --git a/openlibrary/plugins/openlibrary/js/index.js b/openlibrary/plugins/openlibrary/js/index.js index 41b64de97942..b7bc3d5b5d4f 100644 --- a/openlibrary/plugins/openlibrary/js/index.js +++ b/openlibrary/plugins/openlibrary/js/index.js @@ -127,6 +127,7 @@ jQuery(function () { const addRowButton = document.getElementById('add_row_button'); const roles = document.querySelector('#roles'); const identifiers = document.querySelector('#identifiers'); + const workIdentifiers = document.querySelector('#workidentifiers'); const classifications = document.querySelector('#classifications'); const excerpts = document.getElementById('excerpts'); const links = document.getElementById('links'); @@ -162,6 +163,9 @@ jQuery(function () { if (identifiers) { module.initIdentifierValidation(); } + if (workIdentifiers) { + module.initWorkIdentifierValidation(); + } if (classifications) { module.initClassificationValidation(); } diff --git a/openlibrary/plugins/upstream/addbook.py b/openlibrary/plugins/upstream/addbook.py index f1cb074855be..760b1fa89fc6 100644 --- a/openlibrary/plugins/upstream/addbook.py +++ b/openlibrary/plugins/upstream/addbook.py @@ -599,6 +599,9 @@ def save(self, formdata: web.Storage) -> None: edition_data.works = [{'key': self.work.key}] if self.work is not None: + identifiers = work_data.pop('identifiers', []) + self.work.set_identifiers(identifiers) + self.work.update(work_data) saveutil.save(self.work) diff --git a/openlibrary/plugins/upstream/models.py b/openlibrary/plugins/upstream/models.py index 87e2599606ca..cce67bd9faf4 100644 --- a/openlibrary/plugins/upstream/models.py +++ b/openlibrary/plugins/upstream/models.py @@ -17,7 +17,12 @@ from openlibrary.core.models import Image from openlibrary.core import lending -from openlibrary.plugins.upstream.utils import MultiDict, parse_toc, get_edition_config +from openlibrary.plugins.upstream.utils import ( + MultiDict, + parse_toc, + get_edition_config, + get_work_config, +) from openlibrary.plugins.upstream import account from openlibrary.plugins.upstream import borrow from openlibrary.plugins.worksearch.code import works_by_author @@ -561,6 +566,62 @@ def get_covers(self, use_solr=True): else: return [] + def get_identifiers(self): + """Returns (name, value) pairs of all available identifiers.""" + return self._process_identifiers( + get_work_config().identifiers, self.identifiers + ) + + def set_identifiers(self, identifiers): + """Updates the work from identifiers specified as (name, value) pairs.""" + + d = {} + for id in identifiers: + # ignore bad values + if 'name' not in id or 'value' not in id: + continue + name, value = id['name'], id['value'] + if name == 'lccn': + value = normalize_lccn(value) + # `None` in this field causes errors. See #7999. + if value is not None: + d.setdefault(name, []).append(value) + + self.identifiers = {} + + for name, value in d.items(): + self.identifiers[name] = value + + def _process_identifiers(self, config_, values): + id_map = {} + for id in config_: + id_map[id.name] = id + id.setdefault("label", id.name) + id.setdefault("url_format", None) + + d = MultiDict() + + def process(name, value): + if value: + if not isinstance(value, list): + value = [value] + + id = id_map.get(name) or web.storage( + name=name, label=name, url_format=None + ) + for v in value: + d[id.name] = web.storage( + name=id.name, + label=id.label, + value=v, + url=id.get('url') and id.url.replace('@@@', v.replace(' ', '')), + ) + + for name in values: + process(name, values[name]) + + return d + def get_covers_from_solr(self): try: w = self._solr_data diff --git a/openlibrary/plugins/upstream/utils.py b/openlibrary/plugins/upstream/utils.py index 38dc26dda89b..b2144058316b 100644 --- a/openlibrary/plugins/upstream/utils.py +++ b/openlibrary/plugins/upstream/utils.py @@ -848,6 +848,27 @@ def _get_author_config(): return Storage(identifiers=identifiers) +@public +def get_work_config() -> Storage: + return _get_work_config() + + +@web.memoize +def _get_work_config(): + """Returns the work config. + + The results are cached on the first invocation. Any changes to /config/work page require restarting the app. + + This is is cached because fetching and creating the Thing object was taking about 20ms of time for each book request. + """ + thing = web.ctx.site.get('/config/work') + if hasattr(thing, "identifiers"): + identifiers = [Storage(t.dict()) for t in thing.identifiers if 'name' in t] + else: + identifiers = {} + return Storage(identifiers=identifiers) + + @public def get_edition_config() -> Storage: return _get_edition_config() diff --git a/openlibrary/templates/books/edit.html b/openlibrary/templates/books/edit.html index 647afa388fa5..0b5435b48885 100644 --- a/openlibrary/templates/books/edit.html +++ b/openlibrary/templates/books/edit.html @@ -2,6 +2,8 @@ $ this_title = work.title + ': ' + work.subtitle if work.get('subtitle', None) else work.title +$ work_config = get_work_config() + $var title: $this_title $putctx("robots", "noindex,nofollow") @@ -105,6 +107,73 @@

+ $ config = ({ + $ 'Please select an identifier.': _('Please select an identifier.'), + $ 'You need to give a value to ID.': _('You need to give a value to ID.'), + $ 'ID ids cannot contain whitespace.': _('ID ids cannot contain whitespace.'), + $ 'That ID already exists for this work.': _('That ID already exists for this work.'), + $ 'Invalid ID format': _('Invalid ID format') + $ }) +
+ $_("ID Numbers") +
+ + +
+
+ + $_("Like, VIAF?") +
+
+ + + + + + + + + + + + + + + + + + $for i, id in enumerate(work.get_identifiers().values()): + + + + + + +
+ + + + + +
Open Library$work.key.split("/")[-1]
$id_labels.get(id.name, id.name)$id.value + + + [x]
+
+
+
+
$_("Add Excerpts")
diff --git a/openlibrary/templates/type/edition/view.html b/openlibrary/templates/type/edition/view.html index fe00199b64ff..26fc2644f8fc 100644 --- a/openlibrary/templates/type/edition/view.html +++ b/openlibrary/templates/type/edition/view.html @@ -506,6 +506,20 @@

$:_('Links outside Open Library')<
  • $link.title

    • + +
    +

    $_("Work ID Numbers")

    +
    + $:display_identifiers("Open Library", [storage(url=None, value=work.key.split("/")[-1])]) + $for name, values in work.get_identifiers().multi_items(): + $ identifier_label = values[0].label + $:display_identifiers(identifier_label, values) + + $:display_goodreads("Open Library", [storage(url=None, value=work.key.split("/")[-1])]) + $for name, values in work.get_identifiers().multi_items(): + $:display_goodreads(values[0].label, values) +
    +
    $if show_observations: