Refactor/worksearch dry

.pre-commit-config.yaml

@@ -47,7 +47,7 @@ repos:
           - types-all
 
   - repo: https://github.com/asottile/pyupgrade
-    rev: v2.38.0
+    rev: v2.38.2
     hooks:
       - id: pyupgrade
         args:  # Solr on Cython is not yet ready for 3.10 type hints

bundlesize.config.json

@@ -58,7 +58,7 @@
     },
     {
       "path": "static/build/admin.*.js",
-      "maxSize": "1KB"
+      "maxSize": "1.3KB"
     },
     {
       "path": "static/build/search.*.js",

openlibrary/components/MergeUI.vue

@@ -114,6 +114,13 @@ export default {
                 const splitKey = master.key.split('/')
                 const primaryRecord = splitKey[splitKey.length - 1]
                 await createMergeRequest(workIds, primaryRecord, 'create-pending', this.comment)
+                    .then(response => response.json())
+                    .then(data => {
+                        if (data.status === 'ok') {
+                            // Redirect to merge table on success:
+                            window.location.replace(`/merges#mrid-${data.id}`)
+                        }
+                    })
             }
             this.mergeStatus = 'Done';
         },

openlibrary/core/bookshelves.py

@@ -88,6 +88,7 @@ def most_logged_books(
         users. This query is limited to a specific shelf_id (e.g. 1
         for "Want to Read").
         """
+        page = int(page or 1)
         offset = (page - 1) * limit
         oldb = db.get_db()
         where = 'WHERE bookshelf_id' + ('=$shelf_id' if shelf_id else ' IS NOT NULL ')

openlibrary/core/models.py

@@ -646,7 +646,7 @@ def resolve_redirects_bulk(
         start_offset=0,
         grace_period_days=7,
         cutoff_date=datetime.datetime(year=2017, month=1, day=1),
-        test=True,
+        test=False,
     ):
         """
         batch_size - how many records to fetch per batch

openlibrary/core/schema.sql

@@ -1,5 +1,4 @@
 
-
 CREATE TABLE ratings (
     username text NOT NULL,
     work_id integer NOT NULL,
@@ -9,6 +8,7 @@ CREATE TABLE ratings (
     created timestamp without time zone default (current_timestamp at time zone 'utc'),
     primary key (username, work_id)
 );
+CREATE INDEX ratings_work_id_idx ON ratings (work_id);
 
 CREATE TABLE booknotes (
     username text NOT NULL,
@@ -19,6 +19,7 @@ CREATE TABLE booknotes (
     created timestamp without time zone default (current_timestamp at time zone 'utc'),
     primary key (username, work_id, edition_id)
 );
+CREATE INDEX booknotes_work_id_idx ON booknotes (work_id);
 
 CREATE TABLE bookshelves (
     id serial not null primary key,
@@ -39,7 +40,8 @@ CREATE TABLE bookshelves_books (
     created timestamp without time zone default (current_timestamp at time zone 'utc'),
     primary key (username, work_id, bookshelf_id)
 );
-
+CREATE INDEX bookshelves_books_work_id_idx ON bookshelves_books (work_id);
+-- bookshelves_votes currently unused
 CREATE TABLE bookshelves_votes (
     username text NOT NULL,
     bookshelf_id serial NOT NULL REFERENCES bookshelves(id) ON DELETE CASCADE ON UPDATE CASCADE,
@@ -62,6 +64,7 @@ CREATE TABLE observations (
     created timestamp without time zone default (current_timestamp at time zone 'utc'),
     primary key (work_id, edition_id, username, observation_value, observation_type)
 );
+CREATE INDEX observations_username_idx ON observations (username);
 
 CREATE TABLE community_edits_queue (
     id serial not null primary key,

openlibrary/core/sponsorships.py

@@ -103,9 +103,12 @@ def do_we_want_it(isbn):
     }
     url = '%s/book/marc/ol_dedupe.php' % lending.config_ia_domain
     try:
-        data = requests.get(url, params=params).json()
+        data = requests.get(url, params=params, timeout=2).json()
         dwwi = data.get('response', 0)
         return dwwi == 1, data.get('books', [])
+    except requests.exceptions.Timeout:
+        logger.exception('DWWI Timeout')
+        return False, []
     except:
         logger.error("DWWI Failed for isbn %s" % isbn, exc_info=True)
     # err on the side of false negative

openlibrary/plugins/admin/code.py

@@ -82,6 +82,12 @@ def handle(self, cls, args=(), librarians=False):
         if not m:
             raise web.nomethod(cls=cls)
         else:
+            if (
+                context.user
+                and context.user.is_usergroup_member('/usergroup/librarians')
+                and web.ctx.path == '/admin/solr'
+            ):
+                return m(*args)
             if self.is_admin() or (
                 librarians
                 and context.user

openlibrary/plugins/openlibrary/borrow_home.py

@@ -1,4 +1,8 @@
-"""Controller for /borrow page.
+"""
+Controllers for /borrow pages.
+
+These endpoints are largely deprecated, and only maintained for
+backwards compatibility.
 """
 
 import web
@@ -8,227 +12,40 @@
 
 import eventer
 
-from infogami.plugins.api.code import jsonapi
 from infogami.utils import delegate
 from infogami.utils.view import render_template  # noqa: F401 used for its side effects
 
-from openlibrary.core import helpers as h
 from openlibrary.core import statsdb
-from openlibrary.plugins.worksearch.subjects import SubjectEngine
 
 
-class borrow(delegate.page):  # TODO: Why is this function defined twice?
+class borrow(delegate.page):
     path = "/borrow"
 
     def GET(self):
         raise web.seeother('/subjects/in_library#ebooks=true')
 
 
-class borrow(delegate.page):  # type: ignore[no-redef]
+class borrow_json(delegate.page):
     path = "/borrow"
     encoding = "json"
 
-    def is_enabled(self):
-        return "inlibrary" in web.ctx.features
-
-    @jsonapi
     def GET(self):
-        i = web.input(
-            offset=0, limit=12, rand=-1, details="false", has_fulltext="false"
-        )
-
-        filters = {}
-        if i.get("has_fulltext") == "true":
-            filters["has_fulltext"] = "true"
-
-        if i.get("published_in"):
-            if "-" in i.published_in:
-                begin, end = i.published_in.split("-", 1)
-
-                if (
-                    h.safeint(begin, None) is not None
-                    and h.safeint(end, None) is not None
-                ):
-                    filters["publish_year"] = [begin, end]
-            else:
-                y = h.safeint(i.published_in, None)
-                if y is not None:
-                    filters["publish_year"] = i.published_in
+        raise web.seeother('/subjects/in_library.json' + web.ctx.query)
 
-        i.limit = h.safeint(i.limit, 12)
-        i.offset = h.safeint(i.offset, 0)
 
-        i.rand = h.safeint(i.rand, -1)
-
-        if i.rand > 0:
-            sort = 'random_%d desc' % i.rand
-            filters['sort'] = sort
-
-        subject = get_lending_library(
-            web.ctx.site,
-            offset=i.offset,
-            limit=i.limit,
-            details=i.details.lower() == "true",
-            inlibrary=False,
-            **filters,
-        )
-        return json.dumps(subject)
-
-
-class read(delegate.page):  # TODO: Why is this function defined twice?
+class read(delegate.page):
     path = "/read"
 
     def GET(self):
         web.seeother('/subjects/accessible_book#ebooks=true')
 
 
-class read(delegate.page):  # type: ignore[no-redef]
+class read_json(delegate.page):
     path = "/read"
     encoding = "json"
 
-    @jsonapi
-    def GET(self):
-        i = web.input(
-            offset=0, limit=12, rand=-1, details="false", has_fulltext="false"
-        )
-
-        filters = {}
-        if i.get("has_fulltext") == "true":
-            filters["has_fulltext"] = "true"
-
-        if i.get("published_in"):
-            if "-" in i.published_in:
-                begin, end = i.published_in.split("-", 1)
-
-                if (
-                    h.safeint(begin, None) is not None
-                    and h.safeint(end, None) is not None
-                ):
-                    filters["publish_year"] = [begin, end]
-            else:
-                y = h.safeint(i.published_in, None)
-                if y is not None:
-                    filters["publish_year"] = i.published_in
-
-        i.limit = h.safeint(i.limit, 12)
-        i.offset = h.safeint(i.offset, 0)
-
-        i.rand = h.safeint(i.rand, -1)
-
-        if i.rand > 0:
-            sort = 'random_%d desc' % i.rand
-            filters['sort'] = sort
-
-        subject = get_readable_books(
-            web.ctx.site,
-            offset=i.offset,
-            limit=i.limit,
-            details=i.details.lower() == "true",
-            **filters,
-        )
-        return json.dumps(subject)
-
-
-class borrow_about(delegate.page):
-    path = "/borrow/about"
-
     def GET(self):
-        raise web.notfound()
-
-
-def convert_works_to_editions(site, works):
-    """Takes work docs got from solr and converts them into appropriate editions required for lending library."""
-    ekeys = [
-        '/books/' + w['lending_edition'] for w in works if w.get('lending_edition')
-    ]
-    editions = {}
-    for e in site.get_many(ekeys):
-        editions[e['key']] = e.dict()
-
-    for w in works:
-        if w.get('lending_edition'):
-            ekey = '/books/' + w['lending_edition']
-            e = editions.get(ekey)
-            if e and 'ocaid' in e:
-                covers = e.get('covers') or [None]
-                w['key'] = e['key']
-                w['cover_id'] = covers[0]
-                w['ia'] = e['ocaid']
-                w['title'] = e.get('title') or w['title']
-
-
-def get_lending_library(site, inlibrary=False, **kw):
-    kw.setdefault("sort", "first_publish_year desc")
-
-    if inlibrary:
-        subject = CustomSubjectEngine().get_subject(
-            "/subjects/lending_library", in_library=True, **kw
-        )
-    else:
-        subject = CustomSubjectEngine().get_subject(
-            "/subjects/lending_library", in_library=False, **kw
-        )
-
-    subject['key'] = '/borrow'
-    convert_works_to_editions(site, subject['works'])
-    return subject
-
-
-def get_readable_books(site, **kw):
-    kw.setdefault("sort", "first_publish_year desc")
-    subject = ReadableBooksEngine().get_subject("/subjects/dummy", **kw)
-    subject['key'] = '/read'
-    return subject
-
-
-class ReadableBooksEngine(SubjectEngine):
-    """SubjectEngine for readable books.
-
-    This doesn't take subject into account, but considers the public_scan_b
-    field, which is derived from ia collections.
-
-    There is a subject "/subjects/accessible_book", but it has some
-    inlibrary/lendinglibrary books as well because of errors in OL data. Using
-    public_scan_b derived from ia collections is more accurate.
-    """
-
-    def make_query(self, key, filters):
-        return {"public_scan_b": "true"}
-
-    def get_ebook_count(self, name, value, publish_year):
-        # we are not displaying ebook count.
-        # No point making a solr query
-        return 0
-
-
-class CustomSubjectEngine(SubjectEngine):
-    """SubjectEngine for inlibrary and lending_library combined."""
-
-    def make_query(self, key, filters):
-        meta = self.get_meta(key)
-
-        q = {
-            meta.facet_key: ["lending_library"],
-            'public_scan_b': "false",
-            'NOT borrowed_b': "true",
-            # show only books in last 20 or so years
-            'publish_year': (str(1990), str(datetime.date.today().year)),  # range
-        }
-
-        if filters:
-            if filters.get('in_library') is True:
-                q[meta.facet_key].append('in_library')
-            if filters.get("has_fulltext") == "true":
-                q['has_fulltext'] = "true"
-            if filters.get("publish_year"):
-                q['publish_year'] = filters['publish_year']
-
-        return q
-
-    def get_ebook_count(self, name, value, publish_year):
-        # we are not displaying ebook count.
-        # No point making a solr query
-        return 0
+        web.seeother('/subjects/accessible_book.json' + web.ctx.query)
 
 
 def on_loan_created_statsdb(loan):

openlibrary/plugins/openlibrary/js/admin.js

@@ -25,10 +25,27 @@ export function initAdmin() {
 
 export function initAnonymizationButton(button) {
     const displayName = button.dataset.displayName;
-    const confirmMessage = `Really anonymize ${displayName}'s account? This will delete ${displayName}'s profile page and booknotes, and anonymize ${displayName}'s reading log, reviews, and star ratings.`;
+    const confirmMessage = `Really anonymize ${displayName}'s account? This will delete ${displayName}'s profile page and booknotes, and anonymize ${displayName}'s reading log, reviews, star ratings, and merge request submissions.`;
     button.addEventListener('click', function(event) {
         if (!confirm(confirmMessage)) {
             event.preventDefault();
         }
     })
 }
+
+/**
+ * Adds click listener to each given button.  When the button is clicked,
+ * the patron is prompted to confirm the action via a dialog.
+ *
+ * @param {NodeList<HTMLButtonElement>} buttons
+ */
+export function initConfirmationButtons(buttons) {
+    const confirmMessage = 'Are you sure?'
+    for (const button of buttons) {
+        button.addEventListener('click', function(event) {
+            if (!confirm(confirmMessage)) {
+                event.preventDefault();
+            }
+        })
+    }
+}