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

Jump to bottom

Remove jQuery AJAX from the repo-issue.js file #29776

Merged
merged 13 commits into from
Mar 14, 2024
Merged

Remove jQuery AJAX from the repo-issue.js file #29776

Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f4995b8
due-date update and comment deletion
yardenshoham Mar 13, 2024
b92f854
update branch
yardenshoham Mar 13, 2024
02f107c
allow edits from maintainers button
yardenshoham Mar 13, 2024
0c03e33
reviewer addition or deletion
yardenshoham Mar 13, 2024
140dbda
WIP removal button
yardenshoham Mar 13, 2024
42be633
new diff code comment button
yardenshoham Mar 13, 2024
3a3c8fa
issue title edit button
yardenshoham Mar 13, 2024
f67b9d4
Remove CSRF token
yardenshoham Mar 13, 2024
c49a87b
Update web_src/js/features/repo-issue.js
yardenshoham Mar 13, 2024
9bdc5e3
remove double error
yardenshoham Mar 13, 2024
f36afb7
Change call and definition
yardenshoham Mar 13, 2024
72d14d7
Update web_src/js/features/repo-issue.js
yardenshoham Mar 13, 2024
6c6bb4e
Merge branch 'main' into repo-issue-jquery-ajax
yardenshoham Mar 14, 2024
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
198 changes: 114 additions & 84 deletions web_src/js/features/repo-issue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import {setFileFolding} from './file-fold.js';
import {getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js';
import {toAbsoluteUrl} from '../utils.js';
import {initDropzone} from './common-global.js';
import {POST, GET} from '../modules/fetch.js';

const {appSubUrl, csrfToken} = window.config;
const {appSubUrl} = window.config;

export function initRepoIssueTimeTracking() {
$(document).on('click', '.issue-add-time', () => {
Expand Down Expand Up @@ -40,7 +41,7 @@ export function initRepoIssueTimeTracking() {
});
}

function updateDeadline(deadlineString) {
async function updateDeadline(deadlineString) {
hideElem($('#deadline-err-invalid-date'));
$('#deadline-loader').addClass('loading');

Expand All @@ -56,23 +57,21 @@ function updateDeadline(deadlineString) {
realDeadline = new Date(newDate);
}

$.ajax(`${$('#update-issue-deadline-form').attr('action')}`, {
data: JSON.stringify({
due_date: realDeadline,
}),
headers: {
'X-Csrf-Token': csrfToken,
},
contentType: 'application/json',
type: 'POST',
success() {
try {
const response = await POST($('#update-issue-deadline-form').attr('action'), {
data: {due_date: realDeadline}
});

if (response.ok) {
window.location.reload();
},
error() {
$('#deadline-loader').removeClass('loading');
showElem($('#deadline-err-invalid-date'));
},
});
} else {
throw new Error('Invalid response');
}
} catch (error) {
console.error(error);
$('#deadline-loader').removeClass('loading');
silverwind marked this conversation as resolved.
Show resolved Hide resolved
showElem($('#deadline-err-invalid-date'));
}
}

export function initRepoIssueDue() {
Expand Down Expand Up @@ -156,12 +155,12 @@ export function initRepoIssueSidebarList() {

export function initRepoIssueCommentDelete() {
// Delete comment
$(document).on('click', '.delete-comment', function () {
$(document).on('click', '.delete-comment', async function () {
const $this = $(this);
if (window.confirm($this.data('locale'))) {
$.post($this.data('url'), {
_csrf: csrfToken,
}).done(() => {
try {
const response = await POST($this.data('url'));
if (!response.ok) throw new Error('Failed to delete comment');
const $conversationHolder = $this.closest('.conversation-holder');

// Check if this was a pending comment.
Expand All @@ -186,7 +185,9 @@ export function initRepoIssueCommentDelete() {
}
$conversationHolder.remove();
}
});
} catch (error) {
console.error(error);
}
}
return false;
});
Expand Down Expand Up @@ -226,22 +227,32 @@ export function initRepoIssueCodeCommentCancel() {
export function initRepoPullRequestUpdate() {
// Pull Request update button
const $pullUpdateButton = $('.update-button > button');
$pullUpdateButton.on('click', function (e) {
$pullUpdateButton.on('click', async function (e) {
e.preventDefault();
const $this = $(this);
const redirect = $this.data('redirect');
$this.addClass('loading');
$.post($this.data('do'), {
_csrf: csrfToken
}).done((data) => {
if (data.redirect) {
window.location.href = data.redirect;
} else if (redirect) {
window.location.href = redirect;
} else {
window.location.reload();
}
});
let response;
try {
response = await POST($this.data('do'));
} catch (error) {
console.error(error);
} finally {
$this.removeClass('loading');
}
let data;
try {
data = await response?.json(); // the response is probably not a JSON
Copy link
Member

@silverwind silverwind Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Move line into above try imho and remove the ?.

Copy link
Member Author

Choose a reason for hiding this comment

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

The response isn't a JSON though

Copy link
Member

Choose a reason for hiding this comment

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

Can we find out what responses are possible?

Copy link
Member Author

Choose a reason for hiding this comment

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

It returns the full page (HTML)

Copy link
Member

Choose a reason for hiding this comment

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

Always? Then why do we try to parse JSON? :D

Copy link
Member

Choose a reason for hiding this comment

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

Which demons did we summon now?
And why does this sound like HTMX to me?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it but the URL is dynamic and we don't replace content, we reload. It's certainly possible but I'd have to add some form and inputs and have the HX-Refresh header in the response so I decided it's not worth it

} catch (error) {
console.error(error);
}
if (data?.redirect) {
window.location.href = data.redirect;
} else if (redirect) {
window.location.href = redirect;
} else {
window.location.reload();
}
});

$('.update-button > .dropdown').dropdown({
Expand All @@ -267,20 +278,24 @@ export function initRepoPullRequestAllowMaintainerEdit() {

const promptError = $checkbox.attr('data-prompt-error');
$checkbox.checkbox({
'onChange': () => {
'onChange': async () => {
const checked = $checkbox.checkbox('is checked');
let url = $checkbox.attr('data-url');
url += '/set_allow_maintainer_edit';
$checkbox.checkbox('set disabled');
$.ajax({url, type: 'POST',
data: {_csrf: csrfToken, allow_maintainer_edit: checked},
error: () => {
showTemporaryTooltip($checkbox[0], promptError);
},
complete: () => {
$checkbox.checkbox('set enabled');
},
});
try {
const response = await POST(url, {
data: {allow_maintainer_edit: checked},
});
if (!response.ok) {
throw new Error('Failed to update maintainer edit permission');
}
} catch (error) {
console.error(error);
showTemporaryTooltip($checkbox[0], promptError);
} finally {
$checkbox.checkbox('set enabled');
}
},
});
}
Expand Down Expand Up @@ -329,17 +344,15 @@ export function initRepoIssueWipTitle() {
});
}

export async function updateIssuesMeta(url, action, issueIds, elementId) {
return $.ajax({
type: 'POST',
url,
data: {
_csrf: csrfToken,
action,
issue_ids: issueIds,
id: elementId,
},
});
export async function updateIssuesMeta(url, action, issue_ids, id) {
try {
const response = await POST(url, {data: new URLSearchParams({action, issue_ids, id})});
if (!response.ok) {
throw new Error('Failed to update issues meta');
}
} catch (error) {
console.error(error);
}
}

export function initRepoIssueComments() {
Expand Down Expand Up @@ -511,15 +524,20 @@ export function initRepoPullRequestReview() {
const td = ntr.find(`.add-comment-${side}`);
const commentCloud = td.find('.comment-code-cloud');
if (commentCloud.length === 0 && !ntr.find('button[name="pending_review"]').length) {
const html = await $.get($(this).closest('[data-new-comment-url]').attr('data-new-comment-url'));
td.html(html);
td.find("input[name='line']").val(idx);
td.find("input[name='side']").val(side === 'left' ? 'previous' : 'proposed');
td.find("input[name='path']").val(path);

initDropzone(td.find('.dropzone')[0]);
const editor = await initComboMarkdownEditor(td.find('.combo-markdown-editor'));
editor.focus();
try {
const response = await GET($(this).closest('[data-new-comment-url]').attr('data-new-comment-url'));
const html = await response.text();
td.html(html);
yardenshoham marked this conversation as resolved.
Show resolved Hide resolved
td.find("input[name='line']").val(idx);
td.find("input[name='side']").val(side === 'left' ? 'previous' : 'proposed');
td.find("input[name='path']").val(path);

initDropzone(td.find('.dropzone')[0]);
const editor = await initComboMarkdownEditor(td.find('.combo-markdown-editor'));
editor.focus();
} catch (error) {
console.error(error);
}
}
});
}
Expand Down Expand Up @@ -547,11 +565,19 @@ export function initRepoIssueWipToggle() {
const title = toggleWip.getAttribute('data-title');
const wipPrefix = toggleWip.getAttribute('data-wip-prefix');
const updateUrl = toggleWip.getAttribute('data-update-url');
await $.post(updateUrl, {
_csrf: csrfToken,
title: title?.startsWith(wipPrefix) ? title.slice(wipPrefix.length).trim() : `${wipPrefix.trim()} ${title}`,
});
window.location.reload();

try {
const params = new URLSearchParams();
params.append('title', title?.startsWith(wipPrefix) ? title.slice(wipPrefix.length).trim() : `${wipPrefix.trim()} ${title}`);

const response = await POST(updateUrl, {data: params});
if (!response.ok) {
throw new Error('Failed to toggle WIP status');
}
window.location.reload();
} catch (error) {
console.error(error);
}
});
}

Expand All @@ -576,39 +602,43 @@ export function initRepoIssueTitleEdit() {

$('#edit-title').on('click', editTitleToggle);
$('#cancel-edit-title').on('click', editTitleToggle);
$('#save-edit-title').on('click', editTitleToggle).on('click', function () {
const pullrequest_targetbranch_change = function (update_url) {
$('#save-edit-title').on('click', editTitleToggle).on('click', async function () {
const pullrequest_targetbranch_change = async function (update_url) {
const targetBranch = $('#pull-target-branch').data('branch');
const $branchTarget = $('#branch_target');
if (targetBranch === $branchTarget.text()) {
window.location.reload();
return false;
}
$.post(update_url, {
_csrf: csrfToken,
target_branch: targetBranch
}).always(() => {
try {
await POST(update_url, {data: new URLSearchParams({target_branch: targetBranch})});
} catch (error) {
console.error(error);
} finally {
window.location.reload();
});
}
};

const pullrequest_target_update_url = $(this).attr('data-target-update-url');
if ($editInput.val().length === 0 || $editInput.val() === $issueTitle.text()) {
$editInput.val($issueTitle.text());
pullrequest_targetbranch_change(pullrequest_target_update_url);
await pullrequest_targetbranch_change(pullrequest_target_update_url);
} else {
$.post($(this).attr('data-update-url'), {
_csrf: csrfToken,
title: $editInput.val()
}, (data) => {
try {
const params = new URLSearchParams();
params.append('title', $editInput.val());
const response = await POST($(this).attr('data-update-url'), {data: params});
const data = await response.json();
$editInput.val(data.title);
$issueTitle.text(data.title);
if (pullrequest_target_update_url) {
pullrequest_targetbranch_change(pullrequest_target_update_url); // it will reload the window
await pullrequest_targetbranch_change(pullrequest_target_update_url); // it will reload the window
} else {
window.location.reload();
}
});
} catch (error) {
console.error(error);
}
}
return false;
});
Expand Down