search

davydovanton / sidekiq-statistic

See statistic about your workers
MIT License
797 stars 84 forks source link

Fix jQuery missing issues for newer Sidekiq versions #181

Closed hschne closed 2 years ago

hschne commented 2 years ago

First of all, let me say that I really love this gem, its super neat! :100:

I do have some trouble using it at the moment, however. When using sidekiq-statistic with Sidekiq 6.3 or later, it will be broken, as Sidekiq itself no longer ships with jQuery (see Changelog). This was also brought up in #180.

Reproducing

Add sidekiq 6.3 or later and sidekiq-statistic to your gem file. Open the Sidekiq UI, head to the Statistic tab and observe errors in the console. Functionality such as graphs, polling or date picker will not work.

Fix

First, because it was doable, realtime_statistic.js was moved off jQuery by replacing it with plain JS. If Sidekiq can do it, so can we! :stuck_out_tongue:

Unfortunately, doing the same for statistic.js was not really feasible, as that would mean adding a new date picker and so on, and that is kind of a big change. Instead, I opted for loading Sidekiq from a CDN only if Sidekiq 6.3 or later is used.

Let me know if you need any additional changes. I'd be happy to open any follow up PRs to also move statistic.js away from jQuery, if you have any ideas there :slightly_smiling_face:

wenderjean commented 2 years ago

Sorry for the big delay and thanks for your contribution @hschne, I'll take a look and merge asap.

wenderjean commented 2 years ago

Closes https://github.com/davydovanton/sidekiq-statistic/issues/180

hschne commented 2 years ago

Hey @wenderjean , sorry for the delay on my part, I took a bit of a longer vacation :beach_umbrella:

I think everything should be in order now. If you are planning to properly move the other files off JQuery too I'd be happy to lend a hand :slightly_smiling_face:

wenderjean commented 2 years ago

@hschne I created this other issue to track JQuery removal on statistic.js, fell free to leave it a comment and I can assign it to you.