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 rss feed item guid #1536

Merged
merged 5 commits into from
Jan 31, 2024
Merged

fix rss feed item guid #1536

merged 5 commits into from
Jan 31, 2024

Conversation

mcnesium
Copy link
Contributor

I am trying to use the RSS feed feature of this application. For example, it could be useful for embedding it in a Matrix team collaboration room so that everyone in the room can immediately see when a team member has worked on an item. In less busy environments, it might also help to subscribe to the feed in your regular everyday feed reader and be notified of activity that way. Both help to avoid having to actively check the activity application in the Nextcloud browser interface.

I have been doing this for a couple of weeks now, but I have been very unhappy with the output so far. The RSS feed items just made no sense. After a file had been created or edited, file modification notices would appear at what seemed to me to be random times, they would reappear for days on end, even though no one had touched the file. I didn't file a bug about this because I just couldn't describe what was wrong. I could not see any patterns in the feed output and it was very difficult to figure out which RSS feed item was actually caused by which action on a file.

Then I got the idea to test the feed for validity, and voila, the W3C Feed Validator was unhappy with the item's guid:

So I checked a Wordpress feed and found that they validate fine. The relevant difference is that the contents of the guid element had a URL before an actual ID. Furthermore, in the RSS specification the guid has a URL part, too. So I edited templates/rss.php and literally hardcoded some URL into it. I then monitored the output of my Matrix RSS feed bot and my regular feed reader. Suddenly, not only did they match, they even made sense :D

I can definitely not explain, why it is better with the URL part than just the activity_id in the guid element, but now it works for me.

This PR does not have a hardcoded URL in it but takes the p($_['rssLink']); function that is already in use in that template. The output now looks sort of like this:

      <guid isPermaLink="false">https://cloud.domain.de/apps/activity/rss.php?id=3720675</guid>

While that URL does not really lead anywhere, it does fix the weirdness of the RSS feed output in general. It could be improved by actually pointing to a visible event item in the browser frontend of this app, e.g. …/apps/activity?id=3720675. But that would require some <a id="[activity_id]">… HTML in the browser app template plus some function that returns that URL. However, I couldn't easily find out how to do this, also it would probably be beyond the scope of this PR.

@susnux
Copy link
Contributor

susnux commented Jan 24, 2024

It could be improved by actually pointing to a visible event item in the browser frontend of this app, e.g. …/apps/activity?id=3720675.

Sounds like a nice improvement in the future!

Copy link
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the guid must be globally unique and not only relative to the domain?
Nice find, thanks :)

@artonge
Copy link
Collaborator

artonge commented Jan 24, 2024

Can you fix the DCO ?

Signed-off-by: mcnesium <[email protected]>
templates/rss.php Outdated Show resolved Hide resolved
Co-authored-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: mcnesium <[email protected]>
@mcnesium
Copy link
Contributor Author

Looks like the test needs to be updated as well 🥳

Should I just add the http://nextcloud.org?id= to the string here, here and here?

@susnux
Copy link
Contributor

susnux commented Jan 31, 2024

Should I just add

Yes please :)

auto-merge was automatically disabled January 31, 2024 14:32

Head branch was pushed to by a user without write access

@susnux susnux merged commit 0885be4 into nextcloud:master Jan 31, 2024
43 checks passed
@mcnesium
Copy link
Contributor Author

🎉

@mcnesium mcnesium deleted the fix_guid branch January 31, 2024 15:11
@susnux
Copy link
Contributor

susnux commented Jan 31, 2024

/backport to stable28

@susnux
Copy link
Contributor

susnux commented Jan 31, 2024

/backport to stable27

Copy link

backportbot bot commented Jan 31, 2024

The backport to stable28 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable28
git pull origin stable28

# Create the new backport branch
git checkout -b backport/1536/stable28

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 5a08515a7ce0079cd99702f522ef171d814527b2 2cc730e0f628cb9a138bb4b0352dcda7a73114be c26241ac4de163e9a28eb7fe16b692ea6266ded7

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/1536/stable28

Error: Failed to create pull request: Failed to request reviews: Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the nextcloud/activity repository.


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

Copy link

backportbot bot commented Jan 31, 2024

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b backport/1536/stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 5a08515a7ce0079cd99702f522ef171d814527b2 2cc730e0f628cb9a138bb4b0352dcda7a73114be c26241ac4de163e9a28eb7fe16b692ea6266ded7

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/1536/stable27

Error: Failed to create pull request: Failed to request reviews: Reviews may only be requested from collaborators. One or more of the users or teams you specified is not a collaborator of the nextcloud/activity repository.


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@susnux
Copy link
Contributor

susnux commented Jan 31, 2024

/backport to stable28

Copy link

backportbot bot commented Jan 31, 2024

The backport to stable28 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable28
git pull origin stable28

# Create the new backport branch
git checkout -b backport/1536/stable28

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 5a08515a7ce0079cd99702f522ef171d814527b2 2cc730e0f628cb9a138bb4b0352dcda7a73114be c26241ac4de163e9a28eb7fe16b692ea6266ded7

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/1536/stable28

Error: Failed to create pull request: Validation Failed: {"resource":"PullRequest","code":"custom","message":"A pull request already exists for nextcloud:backport/1536/stable28."}


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

Copy link
Contributor

github-actions bot commented Feb 7, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants