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

Fix AttributeError: 'author_names' #4432

Merged
merged 1 commit into from
Jan 18, 2021
Merged

Conversation

@cclauss
Copy link
Contributor

cclauss commented Jan 18, 2021

For the case where this code is currently failing, page.key.startswith('/????')?

I find openlibrary/templates/type/edition/view.html to be nasty code. Every time someone touches it, those changes break something else. My sense is this file gets heavy user traffic yet its logic is muddled. I view the file as a candidate for a rewrite. We should have separate logic flows for page.key.startswith('/books'), page.key.startswith('/works'), etc. to remove the current patches on patches. What are all the page.keys that this code needs to support? Do all page types have a page.author_names attribute?

@SaravgiYash
Copy link
Contributor Author

I find openlibrary/templates/type/edition/view.html to be nasty code. Every time someone touches it, those changes break something else. My sense is this file gets heavy user traffic yet its logic is muddled. I view the file as a candidate for a rewrite. We should have separate logic flows for page.key.startswith('/books'), page.key.startswith('/works'), etc. to remove the current patches on patches. What are all the page.keys that this code needs to support? Do all page types have a page.author_names attribute?

@cclauss we should also focus on documentation (for instance, this particular file has 0 comments).

@jdlrobson
Copy link
Collaborator

Can confirm this patch fixes the error. Totally agree that openlibrary/templates/type/edition/view.html is long overdue a rewrite.

@jdlrobson jdlrobson self-requested a review January 18, 2021 19:11
@cclauss cclauss self-assigned this Jan 18, 2021
@cclauss cclauss merged commit 0cbc8fc into internetarchive:master Jan 18, 2021
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 5, 2021
Sabreen-Parveen pushed a commit to Sabreen-Parveen/openlibrary that referenced this pull request Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants