Stop query if user Id & api not set. Slowed down no chat windows notifications #39
Conversation
Fixed small bug
|
Have asked @Alys in the guild on the best method of testing this. |
I forgot!
|
Thanks for your patience with the multiple commits. I forgot to update the firefox zip and only realised when getting ready for bed. Should be done now till the testers have completed testing. |
2.1.1 was taken on server release. (Look at me I remember the zip file this time too)
|
Done. Updated the mainfest, zip and Readme |
|
Just a little update, I have been running some testing on the refresh rates on chat and have found if you extend the chat greater than 60 minutes it is still querying every 5 seconds if you just leave your computer open and the chat focused. I think I will need to adjust this. @Alys et all, could you confirm are you happy with the refresh rates for timeouts less than 60 minutes?
I am planning to change the code if the timeout is greater than 60 minutes that it will proportionally increase the timeouts. Ie if you change the timeout to 120 minutes the RefreshRateFast will be every 10 seconds. |
|
5 seconds is too frequent unfortunately, refresh rates should be of at
least 60s
Il Mar 12 Mar 2019, 00:25 cTheDragons <notifications@github.com> ha scritto:
… Just a little update, I have been running some testing on the refresh
rates on chat and have found if you extend the chat greater than 60 minutes
it is still querying every 5 seconds if you just leave your computer open
and the chat focused. I think I will need to adjust this.
@Alys <https://github.com/Alys> et all, could you confirm are you happy
with the refresh rates for timeouts less than 60 minutes?
- 5 seconds if the chat windows is focus (RefreshRateFast)
- 45 seconds if the chat window is minmised (RefreshRateMedium) *I
think*
- 60 seconds if the chat window not focused (RefreshRateSlow)
- 40 seconds if there is no chat windows.
I am planning to change the code if the timeout is greater than 60 minutes
that it will proportionally increase the timeouts. Ie if you change the
timeout to 120 minutes the RefreshRateFast will be every 10 seconds.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAkAV_EHvr-Bpx8Zks-vUR71GJ7XAGe4ks5vVuYHgaJpZM4bomgx>
.
|
|
Really? It has been like this for a long time. I think since the beginning... Should there be some API limiting if 5s calls are too much. |
|
Eh, yeah there should be some rate limiting but right now we don't have
anything in place.
So in the meantime 5s is way too often given the server problems we're
having. Habitica chat is not meant to be realtime, I understand 60s might
be a little too slow, so let's make it 30s minimum if you want
(I also noticed you poll the entire user every time, would it be possible
to only fetch the notifications?)
Il Mar 12 Mar 2019, 02:03 cTheDragons <notifications@github.com> ha scritto:
… Really?
It has been like this for a long time
<https://github.com/HabitRPG/habitica-chat-extension/blob/a71dddd0246dd89b0af016dca7c9216d9d12deb7/mainChat/chat_inPage.js#L386>.
I think since the beginning
<https://github.com/HabitRPG/habitica-chat-extension/blob/51b6389c1a7c08fff617f6145c449a2792834b4a/chat_inPage.js#L319>
...
Should there be some API limiting if 5s calls are too much.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAkAV_vy4DV7IArcwPgkWeN3YYE4Yusqks5vVvz9gaJpZM4bomgx>
.
|
|
Will do. How do you fetch notifications? |
|
Sorry, I thought there was a route to only read notifications but looks
like I got confused!
Il Mar 12 Mar 2019, 09:32 cTheDragons <notifications@github.com> ha scritto:
… Will do. How do you fetch notifications?
The API Doc only has posts options
<https://habitica.com/apidoc/#api-Notification-ReadNotifications>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAkAV6yuN7WsCS-SvZfFUXDTQdoE7jLsks5vV2Y7gaJpZM4bomgx>
.
|
|
Is there are nice route other than user that will allow me to read notifications other than user? |
|
No but you can use the userFields parameter to only get the needed fields
instead of the entire user
Il Mar 12 Mar 2019, 09:42 cTheDragons <notifications@github.com> ha scritto:
… Is there are nice route other than user that will allow me to read
notifications other than user?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAkAVzsRdqN-yeSyVtwv41SJGf-g_YSVks5vV2hcgaJpZM4bomgx>
.
|
+ Slow down refresh rate to 30s as requested + Better poll checking if userID and token is 36 characters but still wrong + Fixing minor issues with firefox. (chat_inPage.js is the same as chrome)
Yes I forgot again.
|
All done. (Including the readme file)
Will submit it back to the guild for testing. |
|
@paglias there's a group of up to 15 socialites and mods who are available to test this. Is it okay if testing happens now or do you want to set up any monitoring in advance, or wait for anything else? |
|
Testing now is fine
Il Mar 12 Mar 2019, 12:21 Alys <notifications@github.com> ha scritto:
… @paglias <https://github.com/paglias> there's a group of up to 15
socialites and mods who are available to test this. Is it okay if testing
happens now or do you want to set up any monitoring in advance, or wait for
anything else?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAkAV7A4kuuj5KAkpX-NeBJV5qPLoGp5ks5vV43JgaJpZM4bomgx>
.
|
|
This can be released. Confirmed by the following users:
Let me know if you require anything else. Thanks |
|
thanks @cTheDragons ! There's just one thing we'd like you to add before this can be released: when making calls to the API, could you pass another header (alongside the x-api-user and x-api-key) to make it easier to identify requests coming from the chat extension? Something like |
|
Actually regarding the header name, let's use |
Fix internal error with party notificaiton. Update Readmefile with changes.
SUCH A PAIN!
* Modify date option to include short date * Minor wording change * Updated version
Fix indentation of display names when avatars are disabled
|
Changes completed. Just waiting on testing to occur with Socialites. |
Updating link
|
Found some issues. These have been corrected. Retesting with 2.2.1. |
|
Version 2.2.1 ready to release. This solves issue #38. Confirmation from
It should be noted that both with @aspiring_advocate on Firefox and @ceran on Chrome both encountered issues
However in both these cases it is believe this was due to server issues at the time, as further tests this did not occurred. (The system no longer re-queries if it cannot load the group list which explains this behaviour). |
|
Thanks @cTheDragons ! Will briefly test locally and release |
Fix for Issue #38