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

Wrong notifications about mentions #21

Closed
JayVii opened this issue Nov 7, 2022 · 16 comments
Closed

Wrong notifications about mentions #21

JayVii opened this issue Nov 7, 2022 · 16 comments
Labels
bug Something isn't working

Comments

@JayVii
Copy link
Contributor

JayVii commented Nov 7, 2022

I just installed 2.0.0-3 and get random notifications of being mentioned in posts where I am not actually mentioned in. Looks like fbec729 is overdoing something there.

@toddsundsted
Copy link
Owner

@JayVii were those posts replies to your post? i added support for replies since sometimes people reply to post but don't mention me. maybe the labeling needs to be adjusted.

@toddsundsted toddsundsted added the bug Something isn't working label Nov 8, 2022
@JayVii
Copy link
Contributor Author

JayVii commented Nov 8, 2022

No, they are not directed at me. As soon as I built the 2.0.0-3 docker image and deployed it, i got multiple notifications per minute. All from posts where someone i follow reponds to somebody else. I was never involved there

@toddsundsted
Copy link
Owner

thanks! ok looking at the logic again. you can delete that rule in the meantime. i will also add some functionality to recreate notifications and the timeline with the fix.

i usually test on my own instance before releasing, but i rushed these fixes out. :-(

@JayVii
Copy link
Contributor Author

JayVii commented Nov 8, 2022

I rolled back to my last working version until I have time to fiddle, so I am good :)

i usually test on my own instance before releasing, but i rushed these fixes out. :-(

No worries, I am happy to run untested builds, try them out and report any potential issues. I have never used crystal before, so I need to get into it before I can properly contribute. Thanks for your effort, I really appreciate that!

@felixkrohn
Copy link

I can confirm this behaviour and am available for further tests as well, if needed.

@JayVii
Copy link
Contributor Author

JayVii commented Nov 12, 2022

FYI, for the meantime, I am running 2.0.0-3 with the rules set from 2.0.0-2. Working great!

@toddsundsted
Copy link
Owner

i haven't been able to reproduce this, either via tests, or by setting up several instances of ktistec. i'm home again after traveling so i'll deply this myself and see if that help.

@toddsundsted
Copy link
Owner

this should be fixed now. i have a couple more related changes to make before it's complete but the underlying bug is fixed.

replied to objects are not always cached locally but the match was succeeding with nothing bound. subsequent conditions then matched and bound values incorrectly.

@toddsundsted toddsundsted reopened this Nov 16, 2022
@toddsundsted
Copy link
Owner

leaving this open, pending confirmations.

@JayVii
Copy link
Contributor Author

JayVii commented Nov 16, 2022

Just built the image and deployed it. Works as it should!

@toddsundsted
Copy link
Owner

thanks @JayVii

i've been running it all day and it seems to fix the problem.

closing.

@toddsundsted
Copy link
Owner

one final note. i have a commit already to go with a migration that will clean up the incorrect notifications. it will drop tomorrow.

@JayVii
Copy link
Contributor Author

JayVii commented Nov 17, 2022

Thank you a lot @toddsundsted!

@felixkrohn
Copy link

Took me a bit to test, but can now also confirm: works as expected 💯
Thanks!

@toddsundsted
Copy link
Owner

toddsundsted commented Nov 17, 2022

i added a migration to automatically clean up the incorrect notifications. if you have lot of messages (almost three years, like i do) it uses too much memory, and you might want to wait until i push a fix for that. both otherwise it will run automatically if you build and run a server from the latest main.

@felixkrohn
Copy link

heads-up for those trying it: make sure you have a backup of your DB before. In my case the container crashed and, after reverting to the former image, all posts, followers and followings were gone so that I had to restore the DB.
Not sure if I did anything wrong?

  from src/env/__libc_start_main.c:95:2 in 'libc_start_main_stage2'
Unhandled exception: more than one row (DB::Error)
  from ???
  from ???

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants