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.

Sign up for GitHub

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

Fix jQuery missing issues for newer Sidekiq versions #181

Merged
merged 3 commits into from
Apr 13, 2022
Merged

Fix jQuery missing issues for newer Sidekiq versions #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5feed2d
Migrate realtime off jquery, fix statistic js
hschne Jan 28, 2022
e6c08f2
Only require jquery if Sidekiq version matches
hschne Jan 28, 2022
df5553c
Add jquery to worker erb
hschne Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 36 additions & 25 deletions lib/sidekiq/statistic/views/realtime_statistic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const BASEURL = window.location.pathname;

const requestJSON = (url, config) =>
$.ajax(
fetch(
`${BASEURL}/${url}`,
{
dataType: "json",
Expand Down Expand Up @@ -44,8 +44,12 @@ const Elements = {
}

const Listeners = {
toggleVisibilityButton: () => $(Elements.toggleVisibilityButton).click(toggleWorkerVisibility),
realtimeToggleButton: () => $(Elements.realtimeToggleButton).click(toggleRealTime)
toggleVisibilityButton: function() {
document.querySelector(Elements.toggleVisibilityButton).addEventListener('click', toggleWorkerVisibility)
},
realtimeToggleButton: function() {
document.querySelector(Elements.realtimeToggleButton).addEventListener('click', toggleRealTime)
},
}

class ChartsUpdateService {
Expand All @@ -67,11 +71,12 @@ const initialize = () => {

const initializeCharts = () => {
requestJSON(Charts.paths.init)
.done(function (response) {
.then(response => response.json())
.then(function (response) {
const options = Charts.options(response);

Data.failedChart = c3.generate($.extend(options, { bindto: '.realtime__failed-chart' }));
Data.passedChart = c3.generate($.extend(options, { bindto: '.realtime__passed-chart' }));
Data.failedChart = c3.generate({ ...options, ...{ bindto: '.realtime__failed-chart' } });
Data.passedChart = c3.generate({ ...options, ...{ bindto: '.realtime__passed-chart' } });

ChartsUpdateService.start()
})
Expand All @@ -83,31 +88,31 @@ const setEventListeners = () => {
})
}

function toggleWorkerVisibility() {
const toggleWorkerButton = $(this)
function toggleWorkerVisibility(event) {
const toggleWorkerButton = event.target
const { name } = this

const visibilityIcon = toggleWorkerButton.find('.worker__visibility-icon')
const currentStatus = toggleWorkerButton.data('visible')
const visible = !currentStatus

visibilityIcon.toggleClass(`${visibilityStatusClasses[currentStatus]} ${visibilityStatusClasses[visible]}`)

toggleWorkerButton.data('visible', visible)
const currentStatus = toggleWorkerButton.dataset.visible;
const visible = !currentStatus;
const visibilityIcon = toggleWorkerButton.querySelector('.worker__visibility-icon');
if(visibilityIcon) {
visibilityIcon.classList.toggle(visibilityStatusClasses[currentStatus]);
visibilityIcon.classList.toggle(visibilityStatusClasses[visible]);
}
toggleWorkerButton.dataset.visible = visible;

Data.failedChart.toggle(name)
Data.passedChart.toggle(name)

if (!visible) {
Data.excludedWorkers.push(this.name)
} else {
Data.excludedWorkers.splice($.inArray(this.name, Data.excludedWorkers), 1)
Data.excludedWorkers.splice(Data.excludedWorkers.indexOf(this.name), 1)
}
}

function toggleRealTime() {
const toggleRealtimeButton = $(this)

function toggleRealTime(event) {
const toggleRealtimeButton = event.target;
const { start, stop, started } = toggleRealtimeButton.data();

const buttonText = {
Expand All @@ -117,7 +122,7 @@ function toggleRealTime() {

const toggleButton = value => {
toggleRealtimeButton.text(buttonText[value])
toggleRealtimeButton.data('started', value)
toggleRealtimeButton.dataset.started = value
}

if (started) {
Expand All @@ -136,20 +141,26 @@ const visibilityStatusClasses = {
}

const noVisibleWorkers = () =>
$(Elements.toggleVisibilityButton)
.get()
.every(element => $(element).data('visible') === false)
Array
.from(document.querySelectorAll(Elements.toggleVisibilityButton))
.every(element => element.dataset.visible === false)

const updateCharts = () => {
if (noVisibleWorkers()) {
return
}

requestJSON(Charts.paths.realtime, { data: { excluded: Data.excludedWorkers } })
.done(function (data) {
.then(response => response.json())
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems to me we don't exactly need this line here and also the 74. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, could you go into more detail why not? I'm not able to follow.

updateCharts and initializeCharts are two distinct functions, both requesting distinct data. Both responses should arrive as JSON and thus would need to be converted, right? I feel like I may be missing something here 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nevermind @hschne, I supposed the response to be json already, sorry.

.then(function (data) {
Data.failedChart.flow(data['failed'])
Data.passedChart.flow(data['passed'])
})
}

$(initialize);
const docReady = (callback) => {
if (document.readyState !== "loading") callback();
else document.addEventListener("DOMContentLoaded", callback);
}

docReady(() => initialize());
4 changes: 4 additions & 0 deletions lib/sidekiq/statistic/views/statistic.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
<link href="<%= root_path %>ui-datepicker.css" media="screen" rel="stylesheet" type="text/css" />
<link href="<%= root_path %>sidekiq-statistic-light.css" media="screen and (prefers-color-scheme: light)" rel="stylesheet" type="text/css" />
<link href="<%= root_path %>sidekiq-statistic-dark.css" media="screen and (prefers-color-scheme: dark)" rel="stylesheet" type="text/css" />
<% if Sidekiq::VERSION >= '6.3' %>
wenderjean marked this conversation as resolved.
Show resolved Hide resolved
<!-- Jquery was removed in Sidekiq 6.3, so we need to explicitly add it. Several components (e.g. datepicker) require it -->
<script src="https://code.jquery.com/jquery-3.6.0.min.js" integrity="sha256-/xUj+3OJU5yExlq6GSYGSHk7tPXikynS7ogEvDej/m4=" crossorigin="anonymous"></script>
<% end %>
<% end %>

<script type="text/javascript" src="<%= root_path %>c3.js"></script>
Expand Down
5 changes: 4 additions & 1 deletion lib/sidekiq/statistic/views/worker.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
<% add_to_head do %>
<link href="<%= root_path %>common.css" media="screen" rel="stylesheet" type="text/css" />
<link href="<%= root_path %>ui-datepicker.css" media="screen" rel="stylesheet" type="text/css" />

<link href="<%= root_path %>sidekiq-statistic-light.css" media="screen and (prefers-color-scheme: light)" rel="stylesheet" type="text/css" />
<link href="<%= root_path %>sidekiq-statistic-dark.css" media="screen and (prefers-color-scheme: dark)" rel="stylesheet" type="text/css" />
<% if Sidekiq::VERSION >= '6.3' %>
<!-- Jquery was removed in Sidekiq 6.3, so we need to explicitly add it. Several components (e.g. datepicker) require it -->
<script src="https://code.jquery.com/jquery-3.6.0.min.js" integrity="sha256-/xUj+3OJU5yExlq6GSYGSHk7tPXikynS7ogEvDej/m4=" crossorigin="anonymous"></script>
<% end %>
<% end %>

<script type="text/javascript" src="<%= root_path %>statistic.js"></script>
Expand Down