Add author date when showing author name to librarians #9052

Aeltumn · 2024-04-09T11:18:48Z

[feature]

Technical

This is a continuation of #8076. Addresses the comments on this previous PR and completes the feature. Notable changes:

Removed integration test as those have since been removed
Don't show the ( - ) if no birth date is known. It should show when no death date is set, but not if there's no birth date either
Use a dictionary to store the author information and only pass along the dates on the book view
Only show the years if the user is in librarian mode
Added a regex to determine the year, using one to just take the first 4 consequtive digits. This is based on a comment by @cdrini. Is this a fitting regex or did you have another one in mind?

It took some effort to figure out use to run the regex as it needs the re import. So there might have been a simpler way.

Testing

Log in with an account that is in /usergroup/librarian.
Navigate to a random book and check if the author is showing a birth and death year.

Screenshot

Before logging in

After logging in

Stakeholders

@jimchamp

- Store data in dictionaries - Only show years for librarians on book page

for more information, see https://pre-commit.ci

jimchamp

Thanks, @Aeltumn!

This should be good to go after the new suggested changes are implemented.

jimchamp · 2024-04-18T22:08:17Z

openlibrary/core/helpers.py

@@ -53,6 +53,7 @@
    # functions imported from elsewhere
    "parse_datetime",
    "safeint",
+    "extract_year",


Suggested change

"extract_year",

This function was not imported from another file...
Move this line above the "functions imported from elsewhere" comment.

jimchamp · 2024-04-18T22:50:54Z

openlibrary/templates/type/edition/title_and_author.html

@@ -37,11 +37,12 @@ <h2 class="work-subtitle">
    $ author_names = (work or edition).author_names
    $if authors:
      <h2 class="edition-byline">
-        $:macros.BookByline([(author.name, author.url(), author.birth_date, author.death_date) for author in authors], attrs='itemprop="author"')
+        $ is_librarian = ctx.user and (ctx.user.is_librarian() or ctx.user.is_super_librarian())


Suggested change

$ is_librarian = ctx.user and (ctx.user.is_librarian() or ctx.user.is_super_librarian())

$ is_librarian = ctx.user and ctx.user.is_librarian()

All members of /usergroup/super-librarians are members of /usergroup/librarians.

Edit: I see that is not the case in our local environment. Make this change last, so that you are able to test the other suggested changes in your local environment.

I copied the entire statement from another template, hence the or super_librarian. But I'll delete it from here after I'm done testing.

jimchamp · 2024-04-18T23:21:22Z

openlibrary/templates/books/edit.html

@@ -58,7 +58,7 @@ <h2 class="editFormTitle">$:title</h2>
        <h1 class="editFormBookTitle">$this_title</h1>
        $if authors:
            <h3 class="editFormBookAuthors">
-                $:macros.BookByline([(author.name, author.url(), author.birth_date, author.death_date) for author in authors])
+                $:macros.BookByline([{'name': a.name, 'url': a.url()} for author in authors])


Suggested change

$:macros.BookByline([{'name': a.name, 'url': a.url()} for author in authors])

$:macros.BookByline([{'name': a.name, 'url': a.url()} for a in authors])

This is causing errors in book edit pages. Did you test this? Also, the birth and death dates would be useful to librarians while they are editing.

Managing my changes in git was a bit of a mess, so I must've gotten this mixed up somewhere.
Will fix, add the birth/date date here, and test.

Aeltumn · 2024-04-19T00:27:24Z

I've addressed the comments and re-tested my changes.

jimchamp

Everything seems to be working as expected on testing. Thanks, @Aeltumn.

* adding date of birth and death date in almost every place * adding actual files, not just tests * cleaning test file and making another test * cleaning test file from local tests lines * updating get doce test * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add author date when showing author name to librarians (#9052) * Re-implement changes - Store data in dictionaries - Only show years for librarians on book page * Delete integration test * Use a regex to determine the year to display * Use single quotes * Add helper function to extract a year * Register helper * Final fixes * Small clean-up * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --------- Co-authored-by: Lucas Sales <sales_l@hotmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Aeltumn <12281102+Aeltumn@users.noreply.github.com>

…ve#9052) * Re-implement changes - Store data in dictionaries - Only show years for librarians on book page * Delete integration test * Use a regex to determine the year to display * Use single quotes * Add helper function to extract a year * Register helper * Final fixes * Small clean-up * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Aeltumn and others added 9 commits April 9, 2024 11:09

Re-implement changes

db41b98

- Store data in dictionaries - Only show years for librarians on book page

Delete integration test

5056adf

Use a regex to determine the year to display

6ab5079

Use single quotes

abadad8

Add helper function to extract a year

5b75f64

Register helper

9f52a1f

Final fixes

5730a4c

Small clean-up

88d8930

[pre-commit.ci] auto fixes from pre-commit.com hooks

8dde8c8

for more information, see https://pre-commit.ci

mekarpeles assigned jimchamp Apr 9, 2024

jimchamp changed the title ~~Add author date when showing author name to librarians, closes #7799~~ Add author date when showing author name to librarians Apr 17, 2024

jimchamp requested changes Apr 18, 2024

View reviewed changes

jimchamp added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Apr 18, 2024

Address comments

0da77ce

Aeltumn requested a review from jimchamp April 19, 2024 00:27

jimchamp added On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing and removed Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] labels Apr 19, 2024

jimchamp approved these changes Apr 19, 2024

View reviewed changes

jimchamp merged commit d9fb943 into internetarchive:author-dates Apr 19, 2024
1 check passed

jimchamp mentioned this pull request Apr 19, 2024

Show author's birth, death dates on book page #9126

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add author date when showing author name to librarians #9052

Add author date when showing author name to librarians #9052

Aeltumn commented Apr 9, 2024

jimchamp left a comment

jimchamp Apr 18, 2024

jimchamp Apr 18, 2024

Aeltumn Apr 19, 2024

jimchamp Apr 18, 2024

Aeltumn Apr 19, 2024

Aeltumn commented Apr 19, 2024

jimchamp left a comment

	$ is_librarian = ctx.user and (ctx.user.is_librarian() or ctx.user.is_super_librarian())
	$ is_librarian = ctx.user and ctx.user.is_librarian()

	$:macros.BookByline([{'name': a.name, 'url': a.url()} for author in authors])
	$:macros.BookByline([{'name': a.name, 'url': a.url()} for a in authors])

Add author date when showing author name to librarians #9052

Add author date when showing author name to librarians #9052

Conversation

Aeltumn commented Apr 9, 2024

Technical

Testing

Screenshot

Stakeholders

jimchamp left a comment

Choose a reason for hiding this comment

jimchamp Apr 18, 2024

Choose a reason for hiding this comment

jimchamp Apr 18, 2024

Choose a reason for hiding this comment

Aeltumn Apr 19, 2024

Choose a reason for hiding this comment

jimchamp Apr 18, 2024

Choose a reason for hiding this comment

Aeltumn Apr 19, 2024

Choose a reason for hiding this comment

Aeltumn commented Apr 19, 2024

jimchamp left a comment

Choose a reason for hiding this comment