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

Notified hundreds of times for a single notification #36

Open
georgikoyrushki95 opened this issue Jul 31, 2016 · 6 comments
Open

Notified hundreds of times for a single notification #36

georgikoyrushki95 opened this issue Jul 31, 2016 · 6 comments

Comments

@georgikoyrushki95
Copy link

Hello there,
My team and I made use of your notifications module for a project we work on as part of an internship.
Everything seemed to be perfectly fine until the point where the module began issuing many notifications for the same event (e.g. a completed background task). This stalls up the whole script execution, and the browser freezes.

Have you experienced the same problem? Any help would be deeply appreciated. Please let me know if you need me to elaborate a bit more on our problem.

Thanks for your help in advance!

@v1k45
Copy link
Owner

v1k45 commented Aug 1, 2016

Hi @georgikoyrushki95 ,
Personally, I haven't experienced such issue (yet). Can you give details on reproducing it?

Are you sure that you background task is not triggering the the module to send notification unexpectedly? And about the browser part, what is making the browser freeze? is it the amount of notification to be inserted into DOM or is it a server related issue? How many notifications did you receive to which led the browser to stall?

Thanks for reporting :)

@gaborbk
Copy link

gaborbk commented Aug 1, 2016

Hi @v1k45 ,

I'm a colleague of @georgikoyrushki95 , we've found the cause of the problem: we have included jQuery and the JS file for your project twice, once in our base template and again in the notification one. This caused the browser to freeze every time a new notification was sent.

Also while testing Notify we have noticed that the 'No notifications yet.' message is hard-coded in notification_tags.py and so I had to fork Notify to remove it (having an if request.user.notifications.unread.count != 0 in the template broke the styling of notifications). Could you include an option to not display this in a future version?

EDIT: By broke the notifications I meant that when they initially appear they have default styling. Apparently this was not caused by the template tag but that Notify always used the default template for them when they first appeared (a bug maybe?).

Thanks for the fast reply and the awesome package!

@v1k45
Copy link
Owner

v1k45 commented Aug 2, 2016

Hey @Gystark,

In the next update i'll add a template which will store the empty notifications message instead of doing this.

Apparently this was not caused by the template tag but that Notify always used the default template for them when they first appeared (a bug maybe?).

If by default template you meant the default empty notification message in the notification box, it should be fixed by allowing users to overriding the template that shows that message.

If you meant something else, please clarify. And I'm glad that you found this project useful.

@gaborbk
Copy link

gaborbk commented Aug 3, 2016

Hi,

Sorry for the late answer. I meant that notifications sometimes appeared for the first time with default template styling, regardless of their type so this seems to be a bug.

@v1k45
Copy link
Owner

v1k45 commented Aug 4, 2016

That seems weird. I'll see if I can reproduce it. Thanks.

On 04-Aug-2016 12:33 am, "Gabor Borics-Kurti" [email protected]
wrote:

Hi,

Sorry for the late answer. I meant that notifications sometimes appeared
for the first time with default template styling, regardless of their type
so this seems to be a bug.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHgzfWgf1dFFoRXYmv4YvYG0NSO2ETeNks5qcOX1gaJpZM4JZFyZ
.

@quevon24
Copy link

I guess that you tried to override the box template. Also happened to me, if you have your code like this with bootstrap will repeat the same notification(s) a lot of times:

<a href="{{ notification.actor_url }}">
<div class="notification alert {{ notification.read|yesno:'alert-success,alert-info' }} {{ notification.read|yesno:'read,unread' }} alert-alt" data-nf-id="{{ notification.id }}">
    <small>{{ notification.created|timesince }} ago</small>
    <br>
    {{ notification.actor }} {{ notification.verb }}
</div>
</a>

But if you rewrite it like this it will work:

<a href="{{ notification.actor_url }}" data-nf-id="{{ notification.id }}">
<div class="notification alert {{ notification.read|yesno:'alert-success,alert-info' }} {{ notification.read|yesno:'read,unread' }} alert-alt">
    <small>{{ notification.created|timesince }} ago</small>
    <br>
    {{ notification.actor }} {{ notification.verb }}
</div>
</a>

Don't know why this happens but it's because of that.

Thanks for this awesome module I hope you still support it in the future.

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

No branches or pull requests

4 participants