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.

Sign up for GitHub

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

Refactors and improves the My Books UI architecture #8597

Merged
merged 7 commits into from
Dec 11, 2023
Merged

Refactors and improves the My Books UI architecture #8597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift click to select a range
75f38c5
Refactors and improves the My Books UI architecture
mekarpeles Dec 7, 2023
7bbf270
Fix non-people /lists endpoints
cdrini Dec 7, 2023
1f0b0b6
Make mybooks path regexes consistent
cdrini Dec 8, 2023
5582aa1
Code review and QA fixes
mekarpeles Dec 8, 2023
feaf1d2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 8, 2023
2a751bc
Dissolve some MyBooks methods
cdrini Dec 11, 2023
1d93856
Remove prefix from mybooks page
cdrini Dec 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion openlibrary/macros/databarView.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 27,7 @@
$if edit and not page.is_fake_record():
<div class="editButton">
<!-- FIXME: accesskey / keyboard shortcut needs i18n -->
<a class="linkButton larger" href="$edit_url" title="$_('Edit this template')"
<a class="cta-btn cta-btn--vanilla" href="$edit_url" title="$_('Edit this template')"
data-ol-link-track="CTAClick|Edit" accesskey="e">$_("Edit")</a>
</div>
</div>
25 changes: 14 additions & 11 deletions openlibrary/plugins/openlibrary/lists.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 215,23 @@ def is_enabled(self):
def GET(self, path):
# If logged in patron is viewing their lists page, use MyBooksTemplate
if path.startswith("/people/"):
Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder if we should simply break the path regex into different functions rather than trying to collapse them into one, just a suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Long term these should be two separate classes I reckon. They're pretty different in terms of function.

user = get_current_user()
username = path.split('/')[-1]

if user and user.key.split('/')[-1] == username:
return MyBooksTemplate(username, 'lists').render()
doc = self.get_doc(path)
if not doc:
raise web.notfound()
mb = MyBooksTemplate(username, 'lists')
if not mb.user:
raise web.notfound()

template = render_template(
"lists/lists.html", mb.user, mb.user.get_lists(), show_header=False
)
return mb.render(template=template, header_title=_("Lists"), page=mb.user)
else:
doc = self.get_doc(path)
if not doc:
raise web.notfound()

lists = doc.get_lists()
return self.render(doc, lists)
lists = doc.get_lists()
return render_template("lists/lists.html", doc, lists, show_header=True)

def get_doc(self, key):
if key.startswith("/subjects/"):
Expand All @@ -237,9 243,6 @@ def get_doc(self, key):
else:
return web.ctx.site.get(key)

def render(self, doc, lists):
return render_template("lists/lists.html", doc, lists)


class lists_edit(delegate.page):
path = r"(/people/[^/] )?(/lists/OL\d L)/edit"
Expand Down
14 changes: 10 additions & 4 deletions openlibrary/plugins/upstream/account.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,8 797,10 @@ class import_books(delegate.page):
def GET(self):
user = accounts.get_current_user()
username = user['key'].split('/')[-1]

return MyBooksTemplate(username, 'imports').render()
template = render['account/import']()
return MyBooksTemplate(username, 'imports').render(
header_title=_("Imports and Exports"), template=template
)


class fetch_goodreads(delegate.page):
Expand Down Expand Up @@ -1033,11 1035,15 @@ class account_loans(delegate.page):

@require_login
def GET(self):
from openlibrary.core.lending import get_loans_of_user

user = accounts.get_current_user()
user.update_loan_status()
username = user['key'].split('/')[-1]

return MyBooksTemplate(username, 'loans').render()
mb = MyBooksTemplate(username, 'loans')
docs = get_loans_of_user(user.key)
template = render['account/loans'](user, docs)
return mb.render(header_title=_("Loans"), template=template)


class account_loans_json(delegate.page):
Expand Down
Loading