From fcb995e3b050672e67abf007d9d4693d7ded72e6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 19 Sep 2023 06:40:03 +0000 Subject: [PATCH 1/8] [pre-commit.ci] pre-commit autoupdate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/astral-sh/ruff-pre-commit: v0.0.288 → v0.0.290](https://github.com/astral-sh/ruff-pre-commit/compare/v0.0.288...v0.0.290) --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 47a594220ce..07bfc2dc9d0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: - id: auto-walrus - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.0.288 + rev: v0.0.290 hooks: - id: ruff From ab152af72afd40b036e92dc1189dda32cc459304 Mon Sep 17 00:00:00 2001 From: Eds-Dbug Date: Wed, 20 Sep 2023 12:34:34 -0400 Subject: [PATCH 2/8] Redo PR 8273 --- ...LibrarianQueue.js => MergeRequestTable.js} | 20 +- .../TableHeader.js | 2 +- .../TableRow.js | 2 +- .../js/merge-request-table/index.js | 4 +- openlibrary/plugins/upstream/edits.py | 2 +- .../templates/merge_queue/merge_queue.html | 235 ------------------ .../merge_request_table.html | 64 +++++ .../merge_request_table/table_header.html | 93 +++++++ .../merge_request_table/table_row.html | 78 ++++++ 9 files changed, 250 insertions(+), 250 deletions(-) rename openlibrary/plugins/openlibrary/js/merge-request-table/{LibrarianQueue.js => MergeRequestTable.js} (65%) rename openlibrary/plugins/openlibrary/js/merge-request-table/{LibrarianQueue => MergeRequestTable}/TableHeader.js (98%) rename openlibrary/plugins/openlibrary/js/merge-request-table/{LibrarianQueue => MergeRequestTable}/TableRow.js (99%) delete mode 100644 openlibrary/templates/merge_queue/merge_queue.html create mode 100644 openlibrary/templates/merge_request_table/merge_request_table.html create mode 100644 openlibrary/templates/merge_request_table/table_header.html create mode 100644 openlibrary/templates/merge_request_table/table_row.html diff --git a/openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue.js b/openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable.js similarity index 65% rename from openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue.js rename to openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable.js index 312185bb715..7e850020557 100644 --- a/openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue.js +++ b/openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable.js @@ -2,33 +2,33 @@ * Defines functionality related to librarian request table and header. * * Base template for the table is openlibrary/templates/merge_queue/merge_queue.html - * @module merge-request-table/LibrarianQueue + * @module merge-request-table/MergeRequestTable */ -import TableHeader from './LibrarianQueue/TableHeader' -import { setI18nStrings, TableRow } from './LibrarianQueue/TableRow' +import TableHeader from './MergeRequestTable/TableHeader' +import { setI18nStrings, TableRow } from './MergeRequestTable/TableRow' /** * Class representing the librarian request table. * * @class */ -export default class LibrarianQueue { +export default class MergeRequestTable { /** * Creates references to the table and its header and hydrates each. * - * @param {HTMLElement} librarianRequestTable + * @param {HTMLElement} mergeRequestTable */ - constructor(librarianRequestTable) { + constructor(mergeRequestTable) { /** * The `username` of the authenticated patron, or '' if logged out. * * @param {string} */ - this.username = librarianRequestTable.dataset.username + this.username = mergeRequestTable.dataset.username - const localizedStrings = JSON.parse(librarianRequestTable.dataset.i18n) + const localizedStrings = JSON.parse(mergeRequestTable.dataset.i18n) setI18nStrings(localizedStrings) /** @@ -36,7 +36,7 @@ export default class LibrarianQueue { * * @param {HTMLElement} */ - this.tableHeader = new TableHeader(librarianRequestTable.querySelector('.table-header')) + this.tableHeader = new TableHeader(mergeRequestTable.querySelector('.table-header')) /** * References to each row in the table. @@ -44,7 +44,7 @@ export default class LibrarianQueue { * @param {Array} */ this.tableRows = [] - const rowElements = librarianRequestTable.querySelectorAll('.mr-table-row') + const rowElements = mergeRequestTable.querySelectorAll('.mr-table-row') for (const elem of rowElements) { this.tableRows.push(new TableRow(elem, this.username)) } diff --git a/openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue/TableHeader.js b/openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable/TableHeader.js similarity index 98% rename from openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue/TableHeader.js rename to openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable/TableHeader.js index 5337543ca95..39be68c2b3d 100644 --- a/openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue/TableHeader.js +++ b/openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable/TableHeader.js @@ -1,7 +1,7 @@ /** * Defines functionality related to librarian request table header. * - * @module merge-request-table/LibrarianQueue/TableHeader + * @module merge-request-table/MergeRequestTable/TableHeader */ /** diff --git a/openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue/TableRow.js b/openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable/TableRow.js similarity index 99% rename from openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue/TableRow.js rename to openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable/TableRow.js index c6173108dff..b78e5988c56 100644 --- a/openlibrary/plugins/openlibrary/js/merge-request-table/LibrarianQueue/TableRow.js +++ b/openlibrary/plugins/openlibrary/js/merge-request-table/MergeRequestTable/TableRow.js @@ -1,7 +1,7 @@ /** * Defines functionality related to librarian request table rows. * - * @module merge-request-table/LibrarianQueue/TableRow + * @module merge-request-table/MergeRequestTable/TableRow */ import { claimRequest, commentOnRequest, declineRequest, unassignRequest } from '../MergeRequestService' diff --git a/openlibrary/plugins/openlibrary/js/merge-request-table/index.js b/openlibrary/plugins/openlibrary/js/merge-request-table/index.js index 314af36cefe..a98cff2c265 100644 --- a/openlibrary/plugins/openlibrary/js/merge-request-table/index.js +++ b/openlibrary/plugins/openlibrary/js/merge-request-table/index.js @@ -1,4 +1,4 @@ -import LibrarianQueue from './LibrarianQueue'; +import MergeRequestTable from './MergeRequestTable'; /** * Hydrates given librarian request queue. @@ -6,6 +6,6 @@ import LibrarianQueue from './LibrarianQueue'; * @param {HTMLElement} elem Reference to the queue's root element. */ export function initLibrarianQueue(elem) { - const librarianQueue = new LibrarianQueue(elem) + const librarianQueue = new MergeRequestTable(elem) librarianQueue.initialize() } diff --git a/openlibrary/plugins/upstream/edits.py b/openlibrary/plugins/upstream/edits.py index cd92c00620a..36fb31a314e 100644 --- a/openlibrary/plugins/upstream/edits.py +++ b/openlibrary/plugins/upstream/edits.py @@ -67,7 +67,7 @@ def GET(self): } return render_template( - 'merge_queue/merge_queue', + 'merge_request_table/merge_request_table', total_found, librarians, merge_requests=merge_requests, diff --git a/openlibrary/templates/merge_queue/merge_queue.html b/openlibrary/templates/merge_queue/merge_queue.html deleted file mode 100644 index 7a35dc367ae..00000000000 --- a/openlibrary/templates/merge_queue/merge_queue.html +++ /dev/null @@ -1,235 +0,0 @@ -$def with(total, librarians, merge_requests=None) - -$# total : dict : {"open": int, "closed": int}; The total number of merge requests found for the current mode -$# librarians : dict : {"submitters": list[str], "reviewers": list[str]} -$# merge_requests : list : Merge requests to be displayed in the table - -$ i18n_strings = { - $ 'comment_submission_failure_message': _('Failed to submit comment. Please try again in a few moments.'), - $ 'close_request_comment_prompt': _('(Optional) Why are you closing this request?') - $ } - -$ username = ctx.user and ctx.user.key.split('/')[-1] -$ can_merge = ctx.user and (ctx.user.is_usergroup_member('/usergroup/super-librarians')) - -$ reviewer = query_param('reviewer', None) -$ submitter = query_param('submitter', None) -$ mode = query_param('mode', 'open') -$ order = query_param('order', 'desc') -$ status = query_param('status', None) - -$if submitter: - $ desc = _("Showing %(username)s's requests only.", username=submitter) - $ link_text = _('Show all requests') - $ href = changequery(submitter=None, page=None) -$else: - $ desc = _('Showing all requests.') - $ link_text = _('Show my requests') if username else '' - $ href = changequery(submitter=username, page=None) if username else changequery(submitter=None, page=None) - -
-

$_('Community Edit Requests')

- -
- $if can_merge: - - $if reviewer: - $_("Showing requests reviewed by %(reviewer)s only.", reviewer=reviewer) $_("Remove reviewer filter") - $else: - $_("Show requests that I've reviewed") - - $desc $link_text -
- - $ page = int(input(page=1).page) -
- $:macros.Pager(page, total[mode], results_per_page=25) -
-
-
- $total['open'] $_('Open') - $total['closed'] $_('Closed') -
- $if mode != 'open': - $# Add filter for request status -
$_('Status ▾') - -
-
$_('Submitter ▾') - -
-
$_('Reviewer ▾') - -
-
$_('Sort ▾') - -
-
- - - - $if not merge_requests: - - - - $code: - # Maps request statuses to status dot class names - status_dot_mapping = { - 0: 'dot--closed', - 1: 'dot--open', - 2: 'dot--merged' - } - $for r in merge_requests: - $ request_title = r.get('title', _('An untitled work') if r['mr_type'] != 2 else _('An unnamed author')) - $ comments = r.get('comments', {}).get('comments', []) - $ is_open = r['status'] == 1 - $ review_url = "%s&mrid=%s" % (r['url'], r['id']) - $ is_submitter = username == r['submitter'] - - - - - - - -
$_('No entries here!')
- $# Start : Status dot indicator - $ dot_status_class = status_dot_mapping[r['status']] - $ status = _('Open request') if is_open else _('Closed request') -
- $# End : Status dot indicator - -
- $:_('%(title)s', title=request_title) - - $# Start : Most recent comment preview -
- $ latest_comment = comments[-1]['message'] if comments else _('No comments yet.') - $latest_comment -
- $# End : Most recent comment preview - - $# Start : Comment section - $# XXX : Replace this with a modal form - - $# End : Comment section - - $# Start : Submitted by line - - $:_('MR #%(id)s opened %(date)s by @%(submitter)s', id=r['id'], date=datestr(r["created"]), submitter=r["submitter"]) - $if is_open and is_submitter: - × - - $# End : Submitted by line -
-
- $ show_review_button = is_open and can_merge and not r.get('reviewer', '') -
- $ request_reviewer = r.get('reviewer', '') - $if request_reviewer: - $ request_reviewer = '@' + request_reviewer - $if is_open: - $ icon_classes = 'mr-review-actions__unassign' - $if not can_merge: - $ icon_classes = '%s hidden' % icon_classes - $ icon_content = '×' - $else: - $ icon_classes = 'mr-review-actions__checkmark' - $ icon_content = '✓' - - $request_reviewer - $:icon_content -
- $_('Review') -
- - $len(comments) - - -
-
-
- $:macros.Pager(page, total[mode], results_per_page=25) -
-
diff --git a/openlibrary/templates/merge_request_table/merge_request_table.html b/openlibrary/templates/merge_request_table/merge_request_table.html new file mode 100644 index 00000000000..c27b7f6b2e7 --- /dev/null +++ b/openlibrary/templates/merge_request_table/merge_request_table.html @@ -0,0 +1,64 @@ +$def with(total, librarians, merge_requests=None) + +$# total : dict : {"open": int, "closed": int}; The total number of merge requests found for the current mode +$# librarians : dict : {"submitters": list[str], "reviewers": list[str]} +$# merge_requests : list : Merge requests to be displayed in the table + +$ i18n_strings = { + $ 'comment_submission_failure_message': _('Failed to submit comment. Please try again in a few moments.'), + $ 'close_request_comment_prompt': _('(Optional) Why are you closing this request?') + $ } + +$ username = ctx.user and ctx.user.key.split('/')[-1] +$ can_merge = ctx.user and (ctx.user.is_usergroup_member('/usergroup/super-librarians')) + +$ reviewer = query_param('reviewer', None) +$ submitter = query_param('submitter', None) +$ mode = query_param('mode', 'open') +$ status = query_param('status', None) + +$if submitter: + $ desc = _("Showing %(username)s's requests only.", username=submitter) + $ link_text = _('Show all requests') + $ href = changequery(submitter=None, page=None) +$else: + $ desc = _('Showing all requests.') + $ link_text = _('Show my requests') if username else '' + $ href = changequery(submitter=username, page=None) if username else changequery(submitter=None, page=None) + +
+

$_('Community Edit Requests')

+ +
+ $if can_merge: + + $if reviewer: + $_("Showing requests reviewed by %(reviewer)s only.", reviewer=reviewer) $_("Remove reviewer filter") + $else: + $_("Show requests that I've reviewed") + + $desc $link_text +
+ + $ page = int(input(page=1).page) +
+ $:macros.Pager(page, total[mode], results_per_page=25) +
+
+ $:render_template('merge_request_table/table_header', total, librarians, mode, username, submitter, reviewer, status) + + + $if not merge_requests: + + + + + $for r in merge_requests: + $:render_template('merge_request_table/table_row', r, username, can_merge) + +
$_('No entries here!')
+
+
+ $:macros.Pager(page, total[mode], results_per_page=25) +
+
diff --git a/openlibrary/templates/merge_request_table/table_header.html b/openlibrary/templates/merge_request_table/table_header.html new file mode 100644 index 00000000000..e0591cfc2f6 --- /dev/null +++ b/openlibrary/templates/merge_request_table/table_header.html @@ -0,0 +1,93 @@ +$def with (total, librarians, mode, username, submitter, reviewer, status) +$ order = query_param('order', 'desc') + +
+ $total['open'] $_('Open') + $total['closed'] $_('Closed') +
+ $if mode != 'open': + $# Add filter for request status +
$_('Status ▾') + +
+
$_('Submitter ▾') + +
+
$_('Reviewer ▾') + +
+
$_('Sort ▾') + +
+
+ diff --git a/openlibrary/templates/merge_request_table/table_row.html b/openlibrary/templates/merge_request_table/table_row.html new file mode 100644 index 00000000000..3dc3071cdc7 --- /dev/null +++ b/openlibrary/templates/merge_request_table/table_row.html @@ -0,0 +1,78 @@ +$def with (request, username, can_merge) +$ request_title = request.get('title', _('An untitled work') if request['mr_type'] != 2 else _('An unnamed author')) +$ comments = request.get('comments', {}).get('comments', []) +$ is_open = request['status'] == 1 +$ review_url = "%s&mrid=%s" % (request['url'], request['id']) +$ is_submitter = username == request['submitter'] +$ status_dot_mapping = {0: 'dot--closed', 1: 'dot--open',2: 'dot--merged'} + + + + $# Start : Status dot indicator + $ dot_status_class = status_dot_mapping[request['status']] + $ status = _('Open request') if is_open else _('Closed request') +
+ $# End : Status dot indicator + +
+ $:_('%(title)s', title=request_title) + + $# Start : Most recent comment preview +
+ $ latest_comment = comments[-1]['message'] if comments else _('No comments yet.') + $latest_comment +
+ $# End : Most recent comment preview + + $# Start : Comment section + $# XXX : Replace this with a modal form + + $# End : Comment section + + $# Start : Submitted by line + + $:_('MR #%(id)s opened %(date)s by @%(submitter)s', id=request['id'], date=datestr(request["created"]), submitter=request["submitter"]) + $if is_open and is_submitter: + × + + $# End : Submitted by line +
+ + + $ show_review_button = is_open and can_merge and not request.get('reviewer', '') +
+ $ request_reviewer = request.get('reviewer', '') + $if request_reviewer: + $ request_reviewer = '@' + request_reviewer + $if is_open: + $ icon_classes = 'mr-review-actions__unassign' + $if not can_merge: + $ icon_classes = '%s hidden' % icon_classes + $ icon_content = '×' + $else: + $ icon_classes = 'mr-review-actions__checkmark' + $ icon_content = '✓' + + $request_reviewer + $:icon_content +
+ $_('Review') + + + + $len(comments) + + + + From e8c7a8bbb7d12d5a170693f5857b157ae75464d8 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 20 Sep 2023 16:42:10 +0000 Subject: [PATCH 3/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../templates/merge_request_table/merge_request_table.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlibrary/templates/merge_request_table/merge_request_table.html b/openlibrary/templates/merge_request_table/merge_request_table.html index c27b7f6b2e7..b8c26f3d9b1 100644 --- a/openlibrary/templates/merge_request_table/merge_request_table.html +++ b/openlibrary/templates/merge_request_table/merge_request_table.html @@ -52,7 +52,7 @@

$_('Community Edit Requests')

$_('No entries here!') - + $for r in merge_requests: $:render_template('merge_request_table/table_row', r, username, can_merge) From bacea2d246fc36938de4b3d93ba7d42c4f36d1ca Mon Sep 17 00:00:00 2001 From: Raymond Berger Date: Sat, 23 Sep 2023 19:47:51 +0200 Subject: [PATCH 4/8] Update bundlesize.config.json --- bundlesize.config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundlesize.config.json b/bundlesize.config.json index 7bd63a2b1b7..f30cc13724c 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -70,7 +70,7 @@ }, { "path": "static/build/lists.*.js", - "maxSize": "5.5KB" + "maxSize": "8KB" }, { "path": "static/build/all.js", From 152bf3360a399feba1ada2c0310a5c0240e340b0 Mon Sep 17 00:00:00 2001 From: Samuel Whittenberger Date: Fri, 22 Sep 2023 12:28:58 -0400 Subject: [PATCH 5/8] add import and export option to mybooks dropper --- openlibrary/templates/account/books.html | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/openlibrary/templates/account/books.html b/openlibrary/templates/account/books.html index 73e717a444e..a391f200c5f 100644 --- a/openlibrary/templates/account/books.html +++ b/openlibrary/templates/account/books.html @@ -55,9 +55,19 @@ $ url_prefix = "/people/" + username $ readlog_url = url_prefix + "/books/%s/" -$ options = [(_("Want to Read (%(count)d)", count=shelf_counts['want-to-read']), readlog_url % "want-to-read"), (_("Currently Reading (%(count)d)", count=shelf_counts['currently-reading']), readlog_url % "currently-reading"), (_("Already Read (%(count)d)", count=shelf_counts['already-read']), readlog_url % "already-read"), (_("My Lists (%(count)d)" if owners_page else "Lists (%(count)d)", count=len(lists)) , url_prefix + "/lists")] +$ options = [ +$ (_("Want to Read (%(count)d)", count=shelf_counts['want-to-read']), readlog_url % "want-to-read"), +$ (_("Currently Reading (%(count)d)", count=shelf_counts['currently-reading']), readlog_url % "currently-reading"), +$ (_("Already Read (%(count)d)", count=shelf_counts['already-read']), readlog_url % "already-read"), +$ (_("My Lists (%(count)d)" if owners_page else "Lists (%(count)d)", count=len(lists)) , url_prefix + "/lists") +$ ] $if owners_page: - $ options = [("My Loans", "/account/loans")] + options + [(_("My Notes"), url_prefix + "/books/notes"), (_("My Reviews"), url_prefix + "/books/observations")] + $ options += [ + $ (_("My Loans"), "/account/loans"), + $ (_("My Notes"), url_prefix + "/books/notes"), + $ (_("My Reviews"), url_prefix + "/books/observations"), + $ (_("Imports and Exports"), "/account/import") + $ ]
From acd3116223f8304877f86dfdcf2d0d4f99fe8222 Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Mon, 25 Sep 2023 21:21:51 +0200 Subject: [PATCH 6/8] ruff rule UP007: Use X | Y for type annotations --- openlibrary/solr/data_provider.py | 6 +++--- openlibrary/solr/query_utils.py | 2 +- openlibrary/solr/update_edition.py | 14 +++++++------- openlibrary/solr/update_work.py | 10 +++++----- openlibrary/utils/solr.py | 2 +- pyproject.toml | 4 +--- scripts/solr_builder/solr_builder/fn_to_cli.py | 2 +- scripts/solr_builder/tests/test_fn_to_cli.py | 6 ++++-- scripts/solr_updater.py | 2 +- 9 files changed, 24 insertions(+), 24 deletions(-) diff --git a/openlibrary/solr/data_provider.py b/openlibrary/solr/data_provider.py index 50c25844f8f..ac8ed0c9faa 100644 --- a/openlibrary/solr/data_provider.py +++ b/openlibrary/solr/data_provider.py @@ -129,7 +129,7 @@ class DataProvider: """ def __init__(self) -> None: - self.ia_cache: dict[str, Optional[dict]] = {} + self.ia_cache: dict[str, dict | None] = {} @staticmethod async def _get_lite_metadata(ocaids: list[str], _recur_depth=0, _max_recur_depth=3): @@ -285,7 +285,7 @@ def get_editions_of_work(self, work): """ raise NotImplementedError() - def get_work_ratings(self, work_key: str) -> Optional[WorkRatingsSummary]: + def get_work_ratings(self, work_key: str) -> WorkRatingsSummary | None: raise NotImplementedError() def get_work_reading_log(self, work_key: str) -> WorkReadingLogSolrSummary | None: @@ -318,7 +318,7 @@ async def get_document(self, key): logger.info("get_document %s", key) return self._withKey(key) - def get_work_ratings(self, work_key: str) -> Optional[WorkRatingsSummary]: + def get_work_ratings(self, work_key: str) -> WorkRatingsSummary | None: work_id = int(work_key[len('/works/OL') : -len('W')]) return Ratings.get_work_ratings_summary(work_id) diff --git a/openlibrary/solr/query_utils.py b/openlibrary/solr/query_utils.py index 274052e9766..e255daebbb3 100644 --- a/openlibrary/solr/query_utils.py +++ b/openlibrary/solr/query_utils.py @@ -168,7 +168,7 @@ def luqum_parser(query: str) -> Item: """ tree = parser.parse(query) - def find_next_word(item: Item) -> Optional[tuple[Word, Optional[BaseOperation]]]: + def find_next_word(item: Item) -> tuple[Word, BaseOperation | None] | None: if isinstance(item, Word): return item, None elif isinstance(item, BaseOperation) and isinstance(item.children[0], Word): diff --git a/openlibrary/solr/update_edition.py b/openlibrary/solr/update_edition.py index aa6cd1bb2a0..30aaf548c94 100644 --- a/openlibrary/solr/update_edition.py +++ b/openlibrary/solr/update_edition.py @@ -37,11 +37,11 @@ def key(self): return self.edition['key'] @property - def title(self) -> Optional[str]: + def title(self) -> str | None: return self.get('title') @property - def subtitle(self) -> Optional[str]: + def subtitle(self) -> str | None: return self.get('subtitle') @property @@ -60,7 +60,7 @@ def alternative_title(self) -> set[str]: return result @property - def cover_i(self) -> Optional[int]: + def cover_i(self) -> int | None: return next( (cover_id for cover_id in self.get('covers', []) if cover_id != -1), None ) @@ -83,7 +83,7 @@ def publisher(self) -> list[str]: ) @property - def number_of_pages(self) -> Optional[int]: + def number_of_pages(self) -> int | None: try: return int(self.get('number_of_pages')) or None except (TypeError, ValueError): # int(None) -> TypeErr, int("vii") -> ValueErr @@ -110,11 +110,11 @@ def lccn(self) -> list[str]: return uniq(lccn.strip() for lccn in self.get('lccn', [])) @property - def publish_date(self) -> Optional[str]: + def publish_date(self) -> str | None: return self.get('publish_date') @property - def publish_year(self) -> Optional[int]: + def publish_year(self) -> int | None: if self.publish_date: m = re_year.search(self.publish_date) return int(m.group(1)) if m else None @@ -122,7 +122,7 @@ def publish_year(self) -> Optional[int]: return None @property - def ia(self) -> Optional[str]: + def ia(self) -> str | None: ocaid = self.get('ocaid') return ocaid.strip() if ocaid else None diff --git a/openlibrary/solr/update_work.py b/openlibrary/solr/update_work.py index bd93e3f6144..1443251e4ff 100644 --- a/openlibrary/solr/update_work.py +++ b/openlibrary/solr/update_work.py @@ -48,7 +48,7 @@ data_provider = cast(DataProvider, None) solr_base_url = None -solr_next: Optional[bool] = None +solr_next: bool | None = None def get_solr_base_url(): @@ -187,8 +187,8 @@ def pick_cover_edition(editions, work_cover_id): ) -def pick_number_of_pages_median(editions: list[dict]) -> Optional[int]: - def to_int(x: Any) -> Optional[int]: +def pick_number_of_pages_median(editions: list[dict]) -> int | None: + def to_int(x: Any) -> int | None: try: return int(x) or None except (TypeError, ValueError): # int(None) -> TypeErr, int("vii") -> ValueErr @@ -1245,7 +1245,7 @@ async def update_work(work: dict) -> list[SolrUpdateRequest]: async def update_author( akey, a=None, handle_redirects=True -) -> Optional[list[SolrUpdateRequest]]: +) -> list[SolrUpdateRequest] | None: """ Get the Solr requests necessary to insert/update/delete an Author in Solr. :param akey: The author key, e.g. /authors/OL23A @@ -1553,7 +1553,7 @@ def load_configs( c_host: str, c_config: str, c_data_provider: ( - Union[DataProvider, Literal['default', 'legacy', 'external']] + DataProvider | Literal["default", "legacy", "external"] ) = 'default', ) -> DataProvider: host = web.lstrips(c_host, "http://").strip("/") diff --git a/openlibrary/utils/solr.py b/openlibrary/utils/solr.py index 56a034bf288..1783b089ccb 100644 --- a/openlibrary/utils/solr.py +++ b/openlibrary/utils/solr.py @@ -39,7 +39,7 @@ def get( key: str, fields: list[str] | None = None, doc_wrapper: Callable[[dict], T] = web.storage, - ) -> Optional[T]: + ) -> T | None: """Get a specific item from solr""" logger.info(f"solr /get: {key}, {fields}") resp = self.session.get( diff --git a/pyproject.toml b/pyproject.toml index 8a3ccfa1cf7..aa658cdc1b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -175,10 +175,8 @@ max-statements = 70 "openlibrary/plugins/upstream/borrow.py" = ["BLE001", "E722"] "openlibrary/plugins/upstream/models.py" = ["BLE001"] "openlibrary/plugins/upstream/utils.py" = ["BLE001"] -"openlibrary/solr/*" = ["UP007"] +"openlibrary/solr/solr_types.py" = ["UP007"] "openlibrary/solr/update_work.py" = ["C901", "E722", "PLR0912", "PLR0915"] -"openlibrary/utils/solr.py" = ["UP007"] -"scripts/solr_*" = ["UP007"] "openlibrary/utils/retry.py" = ["BLE001"] "openlibrary/utils/schema.py" = ["PERF402"] "openlibrary/utils/tests/test_retry.py" = ["PT012", "PT017"] diff --git a/scripts/solr_builder/solr_builder/fn_to_cli.py b/scripts/solr_builder/solr_builder/fn_to_cli.py index 62d72f90afb..289062c1341 100644 --- a/scripts/solr_builder/solr_builder/fn_to_cli.py +++ b/scripts/solr_builder/solr_builder/fn_to_cli.py @@ -49,7 +49,7 @@ def __init__(self, fn: typing.Callable): description=docs.split(':param', 1)[0], formatter_class=ArgumentDefaultsHelpFormatter, ) - self.args: typing.Optional[Namespace] = None + self.args: Namespace | None = None for arg in arg_names: optional = arg in defaults cli_name = arg.replace('_', '-') diff --git a/scripts/solr_builder/tests/test_fn_to_cli.py b/scripts/solr_builder/tests/test_fn_to_cli.py index 119492d792b..13995235e47 100644 --- a/scripts/solr_builder/tests/test_fn_to_cli.py +++ b/scripts/solr_builder/tests/test_fn_to_cli.py @@ -33,7 +33,9 @@ def test_parse_docs(self): def test_type_to_argparse(self): assert FnToCLI.type_to_argparse(int) == {'type': int} - assert FnToCLI.type_to_argparse(typing.Optional[int]) == {'type': int} + assert FnToCLI.type_to_argparse(typing.Optional[int]) == { # noqa: UP007 + 'type': int + } assert FnToCLI.type_to_argparse(bool) == { 'type': bool, 'action': BooleanOptionalAction, @@ -43,5 +45,5 @@ def test_type_to_argparse(self): } def test_is_optional(self): - assert FnToCLI.is_optional(typing.Optional[int]) + assert FnToCLI.is_optional(typing.Optional[int]) # noqa: UP007 assert not FnToCLI.is_optional(int) diff --git a/scripts/solr_updater.py b/scripts/solr_updater.py index ef20ea3638b..eec27784fce 100644 --- a/scripts/solr_updater.py +++ b/scripts/solr_updater.py @@ -111,7 +111,7 @@ def read_records(self, max_fetches=10): self.offset = d['offset'] -def find_keys(d: Union[dict, list]) -> Iterator[str]: +def find_keys(d: dict | list) -> Iterator[str]: """ Find any keys in the given dict or list. From 4a8f9ea79e03f8a7b61f88619f9aeeb4f7ecda90 Mon Sep 17 00:00:00 2001 From: jimchamp Date: Mon, 25 Sep 2023 15:38:21 -0700 Subject: [PATCH 7/8] Apply suggestions from code review --- .../merge_request_table.html | 2 +- .../merge_request_table/table_header.html | 20 ++++++++++++------- .../merge_request_table/table_row.html | 13 +++++++++++- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/openlibrary/templates/merge_request_table/merge_request_table.html b/openlibrary/templates/merge_request_table/merge_request_table.html index b8c26f3d9b1..7507d399f33 100644 --- a/openlibrary/templates/merge_request_table/merge_request_table.html +++ b/openlibrary/templates/merge_request_table/merge_request_table.html @@ -54,7 +54,7 @@

$_('Community Edit Requests')

$for r in merge_requests: - $:render_template('merge_request_table/table_row', r, username, can_merge) + $:render_template('merge_request_table/table_row', r, username, can_merge)
diff --git a/openlibrary/templates/merge_request_table/table_header.html b/openlibrary/templates/merge_request_table/table_header.html index e0591cfc2f6..13e479d3fc5 100644 --- a/openlibrary/templates/merge_request_table/table_header.html +++ b/openlibrary/templates/merge_request_table/table_header.html @@ -1,9 +1,17 @@ -$def with (total, librarians, mode, username, submitter, reviewer, status) +$def with (totals, librarians, mode, username, submitter, reviewer, status) +$# totals : dict : {"open": int, "closed": int} : The total number of merge requests found for each mode +$# librarians : dict : {"submitters": list[str], "reviewers": list[str]} : Collection of reviewer and submitter usernames +$# mode : str : "open" | "closed" : Type of requests that are currently being viewed. +$# username : str | None : Username of authenticated patron, or `None` if patron is unauthenticated. +$# submitter : str | None : Submitter's username. Only passed if we are filtering requests by `submitter`. +$# reviewer : str | None : Reviewer's username. Only passed if we are filtering requests by `reviewer`. +$# status : str | None : "0" | "1" | "2" : Represents a request's status. Only passed if we are filtering by `status` + $ order = query_param('order', 'desc')
- $total['open'] $_('Open') - $total['closed'] $_('Closed') + $totals['open'] $_('Open') + $totals['closed'] $_('Closed')
$if mode != 'open': $# Add filter for request status @@ -16,8 +24,7 @@ $ merged_href = changequery(status='2', page=None) if status!='2' else changequery(status=None, page=None) - $_('Merged') - + $_('Merged') $ declined_href = changequery(status='0', page=None) if status!='0' else changequery(status=None, page=None) @@ -79,8 +86,7 @@ $ desc_href = changequery(order='desc', page=None) if order!='desc' else changequery(order=None, page=None) - $_('Newest') - + $_('Newest') $ asc_href = changequery(order='asc', page=None) if order!='asc' else changequery(order=None, page=None) diff --git a/openlibrary/templates/merge_request_table/table_row.html b/openlibrary/templates/merge_request_table/table_row.html index 3dc3071cdc7..c906126f9f1 100644 --- a/openlibrary/templates/merge_request_table/table_row.html +++ b/openlibrary/templates/merge_request_table/table_row.html @@ -1,10 +1,21 @@ $def with (request, username, can_merge) +$# request : dict : Dict containing information about a single merge request. +$# username : str | None : The patron's username, if currently authenticated. `None` if unauthenticated. +$# can_merge : boolean : `True` if patron has merge privileges. + $ request_title = request.get('title', _('An untitled work') if request['mr_type'] != 2 else _('An unnamed author')) $ comments = request.get('comments', {}).get('comments', []) $ is_open = request['status'] == 1 $ review_url = "%s&mrid=%s" % (request['url'], request['id']) $ is_submitter = username == request['submitter'] -$ status_dot_mapping = {0: 'dot--closed', 1: 'dot--open',2: 'dot--merged'} + +$code: + # Maps request statuses to status dot class names + status_dot_mapping = { + 0: 'dot--closed', + 1: 'dot--open', + 2: 'dot--merged' + } From 4a734fbf1f77a91912fb3ab262fc78c0fc4aae89 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 26 Sep 2023 09:03:45 +0200 Subject: [PATCH 8/8] [pre-commit.ci] pre-commit autoupdate (#8335) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/astral-sh/ruff-pre-commit: v0.0.290 → v0.0.291](https://github.com/astral-sh/ruff-pre-commit/compare/v0.0.290...v0.0.291) - [github.com/pre-commit/mirrors-eslint: v8.49.0 → v8.50.0](https://github.com/pre-commit/mirrors-eslint/compare/v8.49.0...v8.50.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d1eba618c2a..26379694fff 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: - id: auto-walrus - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.0.290 + rev: v0.0.291 hooks: - id: ruff @@ -66,7 +66,7 @@ repos: - id: validate-pyproject - repo: https://github.com/pre-commit/mirrors-eslint - rev: v8.49.0 + rev: v8.50.0 hooks: - id: eslint types: [file]