-
Notifications
You must be signed in to change notification settings - Fork 109
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
Provide admins with a way to regenerate their API token #2718
Conversation
… separate profile tab
…efinition in initializer
// Setup JS functions/libraries so that they're available within the js.erb templates | ||
window.$ = jQuery; | ||
window.jQuery = jQuery; | ||
|
||
// Allow js.erb files to access the notificationHelper functions | ||
window.renderAlert = renderAlert; | ||
window.renderNotice = renderNotice; |
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.
I'll need to make a mental-note this is how you can expose stuff like jQuery.
Thanks for getting this hooked up
app/policies/user_policy.rb
Outdated
def refresh_token? | ||
true | ||
end |
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 we want to validate that the user has either permission to use the API or an existing API token?
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.
Yes, the button should not appear unless they're an Org admin but it would make sense to add logic to the policy
<%# TODO: replace this with the notificationHelper.js once we move to Rails 5 %> | ||
var notification = document.getElementById("notification-area"); | ||
notification.append(msg); | ||
notification.classList.remove('hide'); No newline at end of file | ||
renderNotice(msg); |
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.
Great to see these TODO's going away:)
Ok @xsrust I added some permission checks to that policy |
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.
Thanks for updating
Fixes #2410 .
Added ability for user to regenerate their own API token (or create one if it was not initialized for some reason when they became an Org admin). Also updated the application.js to make jQuery and the notificationHelper functions available within the js.erb files.
Changes proposed in this PR: