Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<l:layout permission="${app.ADMINISTER}" norefresh="true" title="${%title}">
<st:include it="${app}" page="sidepanel.jelly" optional="true"/>
<l:main-panel>
<script src="${rootURL}/plugin/folder-auth/js/notification.js"/>
<script src="${rootURL}/plugin/folder-auth/js/addrole.js"/>
<script src="${rootURL}/plugin/folder-auth/js/folders.js"/>
<script src="${rootURL}/plugin/folder-auth/js/managesids.js"/>
Expand Down
22 changes: 11 additions & 11 deletions src/main/webapp/js/addrole.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
const addGlobalRole = () => {
const roleName = document.getElementById('globalRoleName').value;
if (!roleName || roleName.length < 3) {
alert('Please enter a valid name for the role to be added.');
showNotificationERROR('Please enter a valid name for the role to be added.');
return;
}

Expand All @@ -17,7 +17,7 @@ const addGlobalRole = () => {
};

if (response.permissions.length <= 0) {
alert('Please select at least one permission');
showNotificationERROR('Please select at least one permission');
return;
}

Expand All @@ -31,7 +31,7 @@ const addGlobalRole = () => {
const addFolderRole = () => {
const roleName = document.getElementById('folderRoleName').value;
if (!roleName || roleName.length < 3) {
alert('Please enter a valid name for the role to be added');
showNotificationERROR('Please enter a valid name for the role to be added');
return;
}

Expand All @@ -42,12 +42,12 @@ const addFolderRole = () => {
};

if (!response.permissions || response.permissions.length <= 0) {
alert('Please select at least one permission');
showNotificationERROR('Please select at least one permission');
return;
}

if (!response.folderNames || response.folderNames.length <= 0) {
alert('Please select at least one folder on which this role will be applicable');
showNotificationERROR('Please select at least one folder on which this role will be applicable');
return;
}

Expand All @@ -61,7 +61,7 @@ const addFolderRole = () => {
const addAgentRole = () => {
const roleName = document.getElementById('agentRoleName').value;
if (!roleName || roleName.length < 3) {
alert('Please enter a valid name for the role to be added');
showNotificationERROR('Please enter a valid name for the role to be added');
return;
}

Expand All @@ -72,12 +72,12 @@ const addAgentRole = () => {
};

if (!response.permissions || response.permissions.length <= 0) {
alert('Please select at least one permission');
showNotificationERROR('Please select at least one permission');
return;
}

if (!response.agentNames || response.agentNames.length <= 0) {
alert('Please select at least one agent on which this role will be applicable');
showNotificationERROR('Please select at least one agent on which this role will be applicable');
return;
}

Expand All @@ -100,10 +100,10 @@ const sendPostRequest = (postUrl, json) => {

xhr.onload = () => {
if (xhr.status === 200) {
alert('The role was added successfully');
location.reload(); // refresh the page
showNotificationOK('The role was added successfully');
//location.reload(); // refresh the page
Copy link
Member

Choose a reason for hiding this comment

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

we shouldn't leave commented out code in, it looks like the function handles this?

Suggested change
//location.reload(); // refresh the page

Copy link
Author

Choose a reason for hiding this comment

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

I forgot that, removed now

} else {
alert('Unable to add the role\n' + xhr.responseText);
showNotificationERROR('Unable to add the role\n' + xhr.responseText);
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/main/webapp/js/folders.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const getFolders = () => {
};

xhr.onerror = () => {
alert(`Unable to get the list of folders: ${xhr.responseText}`);
showNotificationERROR(`Unable to get the list of folders: ${xhr.responseText}`);
};
};

Expand Down
12 changes: 6 additions & 6 deletions src/main/webapp/js/managesids.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ function assignSid(roleType, roleName, sidInputBoxId) {
request.open('POST', url);
request.onload = () => {
if (request.status === 200) {
alert('Sid added successfully.');
showNotificationOK('Sid added successfully.');
location.reload();
} else {
alert('Unable to remove the sid.' + request.responseText);
showNotificationERROR('Unable to remove the sid.' + request.responseText);
}

};

request.onerror = () => {
alert('Unable to add the sid to the role: ' + request.responseText);
showNotificationERROR('Unable to add the sid to the role: ' + request.responseText);
};

// see addRole.js
Expand Down Expand Up @@ -59,15 +59,15 @@ function removeSid(roleType, roleName, sidInputBoxId) {
request.open('POST', url);
request.onload = () => {
if (request.status === 200) {
alert('Sid removed successfully.');
showNotificationOK('Sid removed successfully.');
location.reload();
} else {
alert('Unable to remove the sid.' + request.responseText);
showNotificationERROR('Unable to remove the sid.' + request.responseText);
}
};

request.onerror = () => {
alert('Unable to remove the sid from the role: ' + request.responseText);
showNotificationERROR('Unable to remove the sid from the role: ' + request.responseText);
};

// see addRole.js
Expand Down
16 changes: 16 additions & 0 deletions src/main/webapp/js/notification.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
function showNotification(message, options, reload = false) {
notificationBar.show(message, options);
if (reload) {
window.setTimeout(function () {
location.reload();
}, notificationBar.DELAY);
}
}

function showNotificationOK(message) {
showNotification(message, notificationBar.OK, true);
Copy link
Member

Choose a reason for hiding this comment

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

There's a small problem with this behaviour. The page only refreshes when the success notification disappears. So for about 2 seconds, the notification shows but the content is not updated. With the alerts, as soon as the user clicked OK, the page was refreshed. Would it be possible to use a modal popup here? Also note that I'm working on #58 which will completely change the UI.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, thanks for your comment. I will search for modal solution, not sure though if its possible or not. For providing similar functionality I added optional reload feature, which waits for notification.DELAY.

About the UI change, of course It is up to you. We can apply this change until the new UI is published.

@timja Do you know any examples for Modal implementation ? Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

better to not refresh the page and to do a live update, or let the user refresh it themselves really.

Copy link
Author

Choose a reason for hiding this comment

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

I think that is topic of the UI change that @AbhyudayaSharma mentioned. Should we decline this request then ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the code will likely still be useful, thanks for doing this!

Copy link
Author

Choose a reason for hiding this comment

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

No problem

}

function showNotificationERROR(message) {
showNotification(message, notificationBar.ERROR);
}