RSS for shelves #3013
RSS for shelves #3013
Conversation
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.
|
@mouse-reeve any idea why the test fails? |
|
@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. |
|
@hughrun - all checks passed - hopefully the commits are not controversial. |
|
@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. |
|
@bookwyrm-social/code-review if someone has time to review this that would be awesome 🙂 |
|
This sounds so useful! I hope somebody has a chance to review it soon. |
|
@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ó <[email protected]>
Co-authored-by: Adeodato Simó <[email protected]>
Co-authored-by: Adeodato Simó <[email protected]>
Co-authored-by: Adeodato Simó <[email protected]>
getting context data no longer seems needed, if it ever was.
|
@dato thanks for the thorough review. I've addressed all comments. |
|
Ah one last thing for a follow-up PR: producing a |
Co-authored-by: Adeodato Simó <[email protected]>
Co-authored-by: Adeodato Simó <[email protected]>
in book_identifiers.html template we support multiple identifiers in order. This commit adopts the same logic and order
|
I've spotted a few issues, review coming shortly. |
If there is no author for a book, render just the title of the book.
Template blocks can't have logic or other blocks inside them. This commit fixes the problem with the Edition template by removing the blocktrans block: original titles and descriptions shouldn't be translated, and this also makes the template simpler and avoids the logic-in-blocks problem. Additionally if an edition is lacking a description, we fall back to the parent_work description, and line breaks have been added where needed. Also adds a 'rel="alternate"' link to shelf pages to aid feed auto-discovery
|
@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. |
|
@bookwyrm-social/code-review can someone please double-check this, since I added the last commit here? |
This seems to correctly implement an RSS feed for shelves, closing #2871,