-
Notifications
You must be signed in to change notification settings - Fork 208
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
Replace $.ajax() with fetch() #5925
Conversation
e838560
to
9aed887
Compare
@@ -23,6 +23,16 @@ function setupForAll() { | |||
}); | |||
} | |||
|
|||
function getCSRFToken() { | |||
return document.querySelector('meta[name="csrf-token"]').content; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it makes more sense to give this element an ID and use document.getElementById
.
9f07f1a
to
0c65ba6
Compare
Looks generally good so from my side you could go ahead and change more places this way. |
4bc8f1a
to
7e08662
Compare
b507723
to
68bc207
Compare
5e63cd6
to
5a2adfb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is still one open inline-comment: https://github.com/os-autoinst/openQA/pull/5925/files#r1775493947
Otherwise this change is good to merge if tests are passing.
I know it's bad but this line needs to be marked as uncoverable (despite you have already marked the statement as uncoverable): https://app.codecov.io/gh/os-autoinst/openQA/pull/5925/blob/t/lib/OpenQA/SeleniumTest.pm#L149 Maybe it makes also sense to use |
@Martchus done - let's hope that I don't have to add an |
@Mergifyio rebase |
✅ Branch has been successfully rebased |
Please merge when you can monitor |
@@ -171,8 +175,9 @@ function updateTestStatus(newStatus) { | |||
reloadBrokenThumbnails(true); | |||
} | |||
}) | |||
.fail(function () { | |||
.catch(error => { | |||
console.log('ERROR: modlist fail'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need this as there is a console.error in the next line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is good to see where it failed and afterwards also what the error was.
The coverage check still fails because |
Ticket: https://progress.opensuse.org/issues/166310 Co-authored-by: Martchus <[email protected]>
Rebased |
Co-authored-by: Martchus <[email protected]>
Ticket: https://progress.opensuse.org/issues/166310