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

New method for checking for new posts #68

Merged
merged 4 commits into from
Feb 4, 2024

Conversation

kavhayushar
Copy link
Collaborator

Restructured the way we look for new posts.
Instead of fetching the same page and then each time resetting the unread location, instead we fetch the 'forum' where this topic resides and then check the post count for the specific topic in question.

Included enhancements;

  • No longer messing with the "View Count" of the topic
  • No longer messing with the "View Count" of images on current page.
  • No longer messing with the "Unread" location for the logged in user. Hence, fewer load on the server.
  • Changed the link to point to #unread instead of sloppy reloading current page and then having the user confused where to look for the new posts.

Future considerations:

  • We only fetch the very first page of the forum to look for the topic assuming that if there is any new post it for sure will show up on the first page. But theoretically it can be on the second page, if we have new posts in more than 50 topics within this forum. We can fetch the second page if the topic is not found on the first page, but currently I don't believe it's needed.
  • Check if we're on a topic page. Currently the script runs on any and every page (not only in this new version, this is an old issue).
  • For logged in users we can look for the red icon indicating new posts instead of parsing the number of amount of posts. This will accommodate for new posts when some of the old ones got deleted. Currently we only look for the total net amount.

Copy link

what-the-diff bot commented Jan 9, 2024

PR Summary

  • New Response Notification Feature Added
    A new feature has been loaded into our forum platform. This feature will keep an eye out for any new responses on a specific thread of interest. Once it spots a new response, it will notify user by modifying the title of the document and by displaying a notification on the page, which also includes a handy reload button.

@kavhayushar
Copy link
Collaborator Author

I did some testing myself but further thorough testing is needed.

@kavhayushar
Copy link
Collaborator Author

The marking posts as unread is quite a heavy load on the server. (Not sure if it's fixed in the newer version of phpbb). It's worth to eliminate it.

Tested even more. Seems to me as perfectly good and ready to merge but would definitely benefit to have another pair of eyes reviewing it.
Anyone?
Ping: @YisroelTech @mordechairoth @mayim-chayim @nefshech @chucem @iveltcoin @ivelt2

@chucem
Copy link
Contributor

chucem commented Jan 14, 2024

i like the idea. Installed on my computer... now will see how it works.

Copy link
Contributor

@chucem chucem left a comment

Choose a reason for hiding this comment

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

great work! (I didn't even know till now that its reading and unreading...).

I have few comments though

extension/js/newResponseNotification.js Outdated Show resolved Hide resolved
extension/js/newResponseNotification.js Outdated Show resolved Hide resolved
extension/js/newResponseNotification.js Show resolved Hide resolved
kavhayushar and others added 3 commits January 16, 2024 16:45
Co-authored-by: חכם ממה נשתנה <94487951+chucem@users.noreply.github.com>
Co-authored-by: חכם ממה נשתנה <94487951+chucem@users.noreply.github.com>
@chucem
Copy link
Contributor

chucem commented Feb 1, 2024

so far It looks like it works great so far @mordechairoth is it possible to merge and publish?

@mordechairoth mordechairoth merged commit bf8aaa8 into mordechairoth:master Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants