RSS for shelves #3013

mattkatz · 2023-09-29T02:02:33Z

This seems to correctly implement an RSS feed for shelves, closing #2871,

Currently can't get the test to succeed - it fails in an unrelated redis error, so pushing this so I can open a draft PR to get advice on a better test.

didn't really need to add the shelf to self and the linter doesn't like seeing it outside of an init or setup.

jaschaurbach · 2023-10-17T18:43:11Z

@mouse-reeve any idea why the test fails?

mattkatz · 2023-10-21T16:13:54Z

@jaschaurbach good news is that it looks to be a very similar error from the failed test on github vs what I saw locally. Somehow I'm not mocking something out well enough and the test is trying to publish things to redis etc - none of that is set up in the test environment so everything goes boom. I hope to revisit this sometime when I can spend time figuring out why my test explodes.

It's frustrating to get the logic working locally but not the test, but for a project like this where it's just... some people ... working on it, I sympathize that there isn't time to triage the work of everyone that drops by with a pull request. If I can fix it, I will. If mouse or someone else competent sees what I'm doing wrong, that's great.

If I just need to pull an RSS feed of everything for my purposes and then filter out to just the to-read items, I can do that on my side.

bookwyrm/tests/views/test_rss_feed.py

mattkatz · 2023-11-05T05:31:21Z

@hughrun - all checks passed - hopefully the commits are not controversial.

hughrun · 2023-11-05T22:44:04Z

@mattkatz the title of this is still 'DRAFT' - is this ready for review or are you still working on it?

mattkatz · 2023-11-06T11:31:28Z

@mattkatz the title of this is still 'DRAFT' - is this ready for review or are you still working on it?

Fixed. This is ready for review.

hughrun · 2023-11-07T04:41:51Z

@bookwyrm-social/code-review if someone has time to review this that would be awesome 🙂

MaggieFero · 2024-03-02T04:18:23Z

This sounds so useful! I hope somebody has a chance to review it soon.

dato

Hello! Thank you for this feature, and apologies the review took so long. I'm sending you my suggestions, minus i18n issues (which I'll do in a second pass).

Most of these suggestions are minor, or self-explanatory. Please let me know what you think.

bookwyrm/urls.py

bookwyrm/views/rss_feed.py

mattkatz · 2024-03-13T01:42:18Z

@dato - I'll take a look. And thank YOU for volunteering time to do this. I know this is real work and effort and I appreciate it. I'm checking it out now.

This will make sure that users see books as they are shelved, which is what would be expected. Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

getting context data no longer seems needed, if it ever was.

mattkatz · 2024-03-13T02:16:12Z

@dato thanks for the thorough review. I've addressed all comments.
I haven't added new tests around ordering - I don't know that it's worth the test load. If it's needed I'd update the test rss to add 2 books, figure out how to add a published date to each, then add the them to the shelf and expect that ordering is maintained.

dato

Looks good to me with the first two suggestions below!

bookwyrm/views/rss_feed.py

bookwyrm/templates/rss/edition.html

dato · 2024-03-15T17:21:33Z

Ah one last thing for a follow-up PR: producing a <link rel="alternate" type="application/rss+xml" href="…" title="…" /> element in the template for public shelves (unlisted too?).

Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

in book_identifiers.html template we support multiple identifiers in order. This commit adopts the same logic and order

dato

Hi again—

I would like to merge soon so I can see some incremental value

Yes, I support this as well.

I think we can merge, with the tiny adjustments below (needed for a green build; I didn't provide good instructions for blocktrans usage, sorry).

bookwyrm/views/rss_feed.py

bookwyrm/templates/rss/edition.html

hughrun · 2024-03-25T01:33:20Z

I've spotted a few issues, review coming shortly.

hughrun

This generally seems to work, just a couple of things to clean up.

bookwyrm/views/rss_feed.py

bookwyrm/templates/rss/edition.html

bookwyrm/views/rss_feed.py

hughrun · 2024-03-25T02:51:35Z

bookwyrm/templates/rss/edition.html

+{% blocktrans trimmed with book_title=obj.title book_author=obj.author_text %}
+  ‘{{ book_title }}’ by {{ book_author }}
+{% endblocktrans %}
+{{obj.description|default:""}}


This shouldn't be a blocker because it seems to be a bit difficult to manage embedded HTML in the Django syndication framework, but I noticed this just runs on without line breaks so it renders like Title of Book by Jo Bloggs Here is a description of the book.

I'm not sure what to do here.

If I could figure out how to trigger a cdata block, we could do something to embed new lines.
This seems like an approach someone got working, if it makes sense to you, I can take a swing at it.
https://nemecek.be/blog/33/how-to-create-rss-feed-with-html-content-in-django

For my purposes, I'm very interested in getting the book ISBN and other identifiers in as raw a format as possible, but I'd like to make sure this feed is useful and looks good for humans in a feed reader.

When I look at how other book tracking sites handle this, they use the cdata block pretty liberally.
They also just introduce custom elements like isbn and asin directly in the feed. Which doesn't seem valid xml but it seems not to blow anything up.
I'm not ready to dedicate myself just yet to implementing a custom feed generator as described in https://docs.djangoproject.com/en/5.0/ref/contrib/syndication/#custom-feed-generators

btw @hughrun - this is listed as a blocking change I think - assuming I'm not misreading githubs UI.

I'll take a look at this shortly.

If there is no author for a book, render just the title of the book.

hughrun · 2024-04-26T05:59:36Z

@mattkatz I've made a PR in your repository with a fix for the template issue, and added a link the head to enable auto-discovery of the feeds.

Sorry, I kind of lead you in the wrong direction a bit with adding logic for missing authors. We can't put logic inside block tags. But when I thought about it more I realised we shouldn't be translating titles and descriptions anyway: e.g. if it's a book in French the title should be in French, not in whatever language I have my device set to display.

If you're happy with my changes just merge them in to your branch and they will flow through to this PR.

mattkatz added 2 commits September 28, 2023 21:49

working rss feed for shelves

856737e

add a failing test for rss feeds for shelves

4ae0dbd

Currently can't get the test to succeed - it fails in an unrelated redis error, so pushing this so I can open a draft PR to get advice on a better test.

mattkatz mentioned this pull request Sep 30, 2023

Rss for shelves bookwyrm-social/documentation#113

Open

mattkatz added 2 commits October 1, 2023 06:04

fix linting issue

c142e38

didn't really need to add the shelf to self and the linter doesn't like seeing it outside of an init or setup.

fixed method without docstring

f665aea

hughrun reviewed Nov 2, 2023

View reviewed changes

bookwyrm/tests/views/test_rss_feed.py Outdated Show resolved Hide resolved

mattkatz added 2 commits November 4, 2023 21:41

Mock activitystreams add_book_statuses_task

339298c

remove duplicate sitesettings error

d3d5f1b

mattkatz changed the title ~~DRAFT: Rss for shelves~~ RSS for shelves Nov 6, 2023

Merge branch 'main' into rss-for-shelves

7a25869

dato self-requested a review March 6, 2024 21:47

dato reviewed Mar 11, 2024

View reviewed changes

mattkatz and others added 6 commits March 12, 2024 21:43

Sort RSS Feed by Shelved date

7b388ae

This will make sure that users see books as they are shelved, which is what would be expected. Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

/rss shouldn't be optional in the rss route

3754af7

Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

/rss shouldn't be optional in the rss route

a2e41fa

Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

Correct docstring on rss feed

975c3ba

Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

Remove extraneous function

e3ca543

getting context data no longer seems needed, if it ever was.

use privacy__in to respect shelf privacy settings

5b8e083

dato approved these changes Mar 15, 2024

View reviewed changes

bookwyrm/views/rss_feed.py Outdated Show resolved Hide resolved

bookwyrm/views/rss_feed.py Outdated Show resolved Hide resolved

bookwyrm/templates/rss/edition.html Outdated Show resolved Hide resolved

Merge branch 'main' into rss-for-shelves

63d6486

mattkatz and others added 4 commits March 18, 2024 21:29

add user name to shelf rss feed title

6976cb4

Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

improve shelf rss description

cd29b44

Co-authored-by: Adeodato Simó <73768+dato@users.noreply.github.com>

add translation usage

1d8bd2b

support same identifiers as book page in rss

09d857e

in book_identifiers.html template we support multiple identifiers in order. This commit adopts the same logic and order

dato approved these changes Mar 19, 2024

View reviewed changes

bookwyrm/views/rss_feed.py Outdated Show resolved Hide resolved

bookwyrm/templates/rss/edition.html Outdated Show resolved Hide resolved

bookwyrm/templates/rss/edition.html Outdated Show resolved Hide resolved

dato added 2 commits March 19, 2024 01:07

Missing brackets and correct blocktrans usage

1e14d63

Drop default filter in favor of per-metadata item conditionals

5340ed3

hughrun requested changes Mar 25, 2024

View reviewed changes

mattkatz added 4 commits April 14, 2024 04:48

remove pure_name since it only applies to statuses

8dc412c

Don't show a colon if there is no author_text

a0d15cc

If there is no author for a book, render just the title of the book.

handle cases where edition has no author

74e2103

use display_name to avoid name rendering as None

98724df

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RSS for shelves #3013

RSS for shelves #3013

mattkatz commented Sep 29, 2023 •

edited

jaschaurbach commented Oct 17, 2023

mattkatz commented Oct 21, 2023

mattkatz commented Nov 5, 2023

hughrun commented Nov 5, 2023

mattkatz commented Nov 6, 2023

hughrun commented Nov 7, 2023

MaggieFero commented Mar 2, 2024

dato left a comment

mattkatz commented Mar 13, 2024

mattkatz commented Mar 13, 2024

dato left a comment •

edited

dato commented Mar 15, 2024

dato left a comment •

edited

hughrun commented Mar 25, 2024

hughrun left a comment

hughrun Mar 25, 2024

mattkatz Apr 14, 2024

mattkatz Apr 16, 2024

hughrun Apr 24, 2024

hughrun commented Apr 26, 2024

RSS for shelves #3013

Are you sure you want to change the base?

RSS for shelves #3013

Conversation

mattkatz commented Sep 29, 2023 • edited

jaschaurbach commented Oct 17, 2023

mattkatz commented Oct 21, 2023

mattkatz commented Nov 5, 2023

hughrun commented Nov 5, 2023

mattkatz commented Nov 6, 2023

hughrun commented Nov 7, 2023

MaggieFero commented Mar 2, 2024

dato left a comment

Choose a reason for hiding this comment

mattkatz commented Mar 13, 2024

mattkatz commented Mar 13, 2024

dato left a comment • edited

Choose a reason for hiding this comment

dato commented Mar 15, 2024

dato left a comment • edited

Choose a reason for hiding this comment

hughrun commented Mar 25, 2024

hughrun left a comment

Choose a reason for hiding this comment

hughrun Mar 25, 2024

Choose a reason for hiding this comment

mattkatz Apr 14, 2024

Choose a reason for hiding this comment

mattkatz Apr 16, 2024

Choose a reason for hiding this comment

hughrun Apr 24, 2024

Choose a reason for hiding this comment

hughrun commented Apr 26, 2024

mattkatz commented Sep 29, 2023 •

edited

dato left a comment •

edited

dato left a comment •

edited