Project RSS feed #1158

azmeuk · 2023-04-22T23:34:42Z

Partially implements #1130, possibly etags and if-modified-since headers are missing, but let's discuss this in #1130

This patch generates a feed for each project, with a dynamic secret part in the URL based on the existing Project.generate_token method. The generated feed passes the w3c validation

<title> and <description> can probably different, but the main data is here. Let me know what you prefer. Also, the content is simple enough so it does not need to be localized. If more information is needed and the feed content does need to be localized, maybe a lang variable can be added to the feed URL.

Maybe it worth adding an icon somewhere? Near the Invite button?
A firefox extension like awesome-rss automatically detects the feed on a project page, so I can live without it.

ihatemoney/templates/project_feed.xml

Glandos · 2023-05-19T14:04:05Z

As mentioned:

However in order to be able to do this, this would be needed to save the last update datetime for projects. This date would be updated at each bill addition/edition, and would be used to generate the ETag and Last-Modified headers.

We are using SQLAlchemy-continuum, so I guess that the last transaction is already accessible. But I don't have time to investigate it right now, it's just an idea.

azmeuk · 2023-05-19T14:12:30Z

I can look at this in another PR once this one is merged if it OK with you.

azmeuk · 2023-07-18T09:33:02Z

@Glandos any chance you might have some time to review this soon 🙏 ?
Do you have any questions about the implementation? If that can help, we can discuss on irc/matrix/whatever or have a call.

almet

Thanks for working on this. It's working the right way to me, as the generated URLs will be able to only access the RSS endpoint.

Before merging, we should IMO decide if we want to integrate etags and if-modified-since, so that what we have in the main branch is feature-complete.

I advocate for the integration of these two =)

almet · 2023-07-24T18:38:01Z

ihatemoney/templates/project_feed.xml

+	xmlns:atom="http://www.w3.org/2005/Atom"
+	>
+    <channel>
+        <title>{{ g.project.name }}</title>


Maybe add "I Hate Money — " as a prefix here maybe, to make it obvious in the feed reader?

I am not sure this is needed. Most readers display the channel name somewhere. Often you can also click select channel to only see the items for that specific channel. Displaying I Hate Money feels redundant to me.

I'm not sure I get it. Are you saying the channel name isn't coming from the <title>tag? If you only have the project name, how do you know it's coming from this software?

Ahh my bad. I though we were talking about the item title, and not the channel title. It makes sense, I fixed that.

ihatemoney/templates/project_feed.xml

ihatemoney/tests/budget_test.py

ihatemoney/models.py

almet · 2023-07-24T19:11:36Z

Maybe it worth adding an icon somewhere? Near the Invite button?

What about adding a link and a button in the settings of the project?

azmeuk · 2023-07-24T21:41:25Z

What about adding a link and a button in the settings of the project?

I added a RSS button next to the CSS and JSON. Not sure about the icon, I used a picture that was already there.

almet

Looks good to me, I added a few other comments after a second pass. Hope you don't mind too much.

Thanks again for the work on this. I'm curious what other people think about etags and so on.

MANIFEST.in

almet · 2023-07-24T23:35:14Z

ihatemoney/templates/project_feed.xml

+	xmlns:atom="http://www.w3.org/2005/Atom"
+	>
+    <channel>
+        <title>{{ g.project.name }}</title>


I'm not sure I get it. Are you saying the channel name isn't coming from the <title>tag? If you only have the project name, how do you know it's coming from this software?

almet · 2023-07-24T23:36:40Z

ihatemoney/tests/budget_test.py

+    >
+    <channel>
+        <title>raclette</title>
+        <description>A simple shared budget manager web application</description>


Well, actually it feels odd to me. You need to state that it's a feed with the latest bills from this project, maybe?

ihatemoney/tests/budget_test.py

azmeuk · 2023-07-25T07:52:04Z

Looks good to me, I added a few other comments after a second pass. Hope you don't mind too much.

Not at all, I understand what maintainership is and why you may be careful.

I'm curious what other people think about etags and so on.

I'll look at this soon.

azmeuk · 2023-07-25T08:24:03Z

@almet to be able to return a 304 HTTP code, with etags/if-modified-since, we need to keep track of the last update datetime for a given project. I suppose this would mean to add a new column in the model, and update this whenever a bill is added/edited/deleted in the project.

Are you OK with that?

almet · 2023-07-25T11:35:14Z

@almet to be able to return a 304 HTTP code, with etags/if-modified-since, we need to keep track of the last update datetime for a given project. I suppose this would mean to add a new column in the model, and update this whenever a bill is added/edited/deleted in the project.

Are you OK with that?

As @Glandos noted earlier, we're using SQLAlchemy-continuum to version the changes, which should make things easier for us.

It took me some time to find out that the Transaction class has a issued_at property which might be useful for us here.

I'm not sure if we have to chose between ETag/If-None-Match and if-modified-since, or if we should do both of them? (seems redundant, but I guess it depends what's implemented in the clients?).

I advocate for using the easier one (unless there is something I don't get), which in this case seems to be if-modified-since.

azmeuk · 2023-07-25T11:41:06Z

Ok thank you for the tips I will look at that. I think both headers can be implemented at a cheap cost.

azmeuk · 2023-07-25T14:58:51Z

I just force-pushed the branch with the integration of the headers.

I added pytest-libfaketime as a dev dependency to mock datetimes. I chose this instead of freezegun because, as freezegun is pure python it would not correctly mock sqlalchemy datetimes, but libfaketime works on a lower level and is able to mock sqlalchemy issue_at attributes (at least without a heavy setup).
libfaketime does not look very active, but this is because it is mainly stable. I maintain the pytest plugin, and in the past the maintainer was welcoming to contributions, so that wouldn't worry me.

I use continuum to find the last datetime edition for a project, and handle if-modified-since, and then I use this date to build a etag to manage if-none-match.

I could not track bills edition that way though, but addition/deletion works fine.

azmeuk · 2023-07-26T20:59:16Z

I just realized that I forgot to send the etag and last-modified headers back. I need to fix that.

azmeuk · 2023-07-27T14:55:43Z

This is better now.

tox.ini

zorun

I really like the approach, thanks a lot for working on this!

I left some comments, I hope you don't mind adding a few more tests ;)

ihatemoney/web.py

ihatemoney/templates/edit_project.html

zorun · 2023-07-27T19:10:41Z

ihatemoney/tests/budget_test.py

Please add a test that checks that we can't access another project with a "feed" token. Our first and only CVE was about unauthorized access by tweaking the project ID, so we definitely want to check that we don't introduce a similar issue for this new route (even if it's read-only).

Two cases are interesting: another project with the same private code, and another project with a different private code. Both should not allow access.

I was thinking of accessing the RSS feed of another project with the RSS token. Your new tests try to use the RSS token as an invite token, which is already well covered by test_invalid_invite_link_with_feed_token.

Ok that makes more sense 😅
I'll fix that.

This is fixed 👍

ihatemoney/models.py

ihatemoney/web.py

almet

LGTM!

ihatemoney/templates/edit_project.html

zorun · 2023-07-28T12:50:20Z

Thanks for the changes, I still have a change request on the tests (testing we can't access other project's RSS feed), otherwise the rest looks good to me!

zorun

Nice work!

almet · 2023-07-28T15:27:23Z

Thanks again for the work on this. Yay!

zorun · 2023-07-29T09:15:08Z

@azmeuk we will do a release soon with this change (I just plan to finish the work on #1204 before that)

azmeuk commented Apr 22, 2023

View reviewed changes

ihatemoney/templates/project_feed.xml Show resolved Hide resolved

azmeuk mentioned this pull request Apr 22, 2023

RSS feed #1130

Closed

azmeuk force-pushed the issue-1130-rss-feed branch from 71b5e14 to e1f5113 Compare April 23, 2023 12:28

almet reviewed Jul 24, 2023

View reviewed changes

almet self-requested a review July 24, 2023 19:12

azmeuk force-pushed the issue-1130-rss-feed branch from e1f5113 to eb2968b Compare July 24, 2023 21:37

almet reviewed Jul 24, 2023

View reviewed changes

azmeuk force-pushed the issue-1130-rss-feed branch from eb2968b to 4649496 Compare July 25, 2023 07:48

azmeuk force-pushed the issue-1130-rss-feed branch from 4649496 to 1c73d9f Compare July 25, 2023 14:47

azmeuk force-pushed the issue-1130-rss-feed branch from 1c73d9f to a422f7b Compare July 25, 2023 15:00

azmeuk force-pushed the issue-1130-rss-feed branch from a422f7b to cd7a2bc Compare July 27, 2023 14:55

azmeuk commented Jul 27, 2023

View reviewed changes

tox.ini Show resolved Hide resolved

zorun requested changes Jul 27, 2023

View reviewed changes

zorun reviewed Jul 27, 2023

View reviewed changes

ihatemoney/web.py Show resolved Hide resolved

azmeuk force-pushed the issue-1130-rss-feed branch 3 times, most recently from 7bd8a49 to 641a456 Compare July 28, 2023 11:57

azmeuk requested a review from zorun July 28, 2023 12:00

almet approved these changes Jul 28, 2023

View reviewed changes

ihatemoney/templates/edit_project.html Outdated Show resolved Hide resolved

azmeuk force-pushed the issue-1130-rss-feed branch from 641a456 to 16378a4 Compare July 28, 2023 12:19

azmeuk force-pushed the issue-1130-rss-feed branch from 16378a4 to bb1eb07 Compare July 28, 2023 12:55

feat: project RSS feed.

8c27afd

azmeuk force-pushed the issue-1130-rss-feed branch from bb1eb07 to 8c27afd Compare July 28, 2023 13:04

zorun approved these changes Jul 28, 2023

View reviewed changes

zorun merged commit 8d4584d into spiral-project:master Jul 28, 2023
15 checks passed

azmeuk deleted the issue-1130-rss-feed branch July 28, 2023 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Project RSS feed #1158

Project RSS feed #1158

azmeuk commented Apr 22, 2023 •

edited

Loading

Glandos commented May 19, 2023

azmeuk commented May 19, 2023

azmeuk commented Jul 18, 2023

almet left a comment

almet Jul 24, 2023

azmeuk Jul 24, 2023

almet Jul 24, 2023

azmeuk Jul 25, 2023

almet commented Jul 24, 2023

azmeuk commented Jul 24, 2023 •

edited

Loading

almet left a comment

almet Jul 24, 2023

almet Jul 24, 2023

azmeuk commented Jul 25, 2023

azmeuk commented Jul 25, 2023

almet commented Jul 25, 2023

azmeuk commented Jul 25, 2023

azmeuk commented Jul 25, 2023 •

edited

Loading

azmeuk commented Jul 26, 2023

azmeuk commented Jul 27, 2023

zorun left a comment

zorun Jul 27, 2023

azmeuk Jul 28, 2023

zorun Jul 28, 2023

azmeuk Jul 28, 2023

azmeuk Jul 28, 2023

almet left a comment

zorun commented Jul 28, 2023

zorun left a comment

almet commented Jul 28, 2023

zorun commented Jul 29, 2023

Project RSS feed #1158

Project RSS feed #1158

Conversation

azmeuk commented Apr 22, 2023 • edited Loading

Glandos commented May 19, 2023

azmeuk commented May 19, 2023

azmeuk commented Jul 18, 2023

almet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

almet commented Jul 24, 2023

azmeuk commented Jul 24, 2023 • edited Loading

almet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azmeuk commented Jul 25, 2023

azmeuk commented Jul 25, 2023

almet commented Jul 25, 2023

azmeuk commented Jul 25, 2023

azmeuk commented Jul 25, 2023 • edited Loading

azmeuk commented Jul 26, 2023

azmeuk commented Jul 27, 2023

zorun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

almet left a comment

Choose a reason for hiding this comment

zorun commented Jul 28, 2023

zorun left a comment

Choose a reason for hiding this comment

almet commented Jul 28, 2023

zorun commented Jul 29, 2023

azmeuk commented Apr 22, 2023 •

edited

Loading

azmeuk commented Jul 24, 2023 •

edited

Loading

azmeuk commented Jul 25, 2023 •

edited

Loading