Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

feat(account): add menu for managing API credentials #191

Closed
wants to merge 3 commits into from

Conversation

antleblanc
Copy link
Contributor

@antleblanc antleblanc commented Apr 12, 2018

pkuhner and others added 2 commits April 9, 2018 19:58
Users can now manage their API credentials:

 - list applications
 - show permissions
 - revoke credentials
feat(account): add menu for managing API credentials
@antleblanc antleblanc self-assigned this Apr 12, 2018
@antleblanc antleblanc changed the title Feature/account credentials feat(account): add menu for managing API credentials Apr 12, 2018
@antleblanc antleblanc force-pushed the feature/account-credentials branch from 997ad85 to d78db32 Compare April 12, 2018 06:44

<div data-wizard-step>

<p data-ng-bind-html="tr('user_credentials_delete_modal_step1_question', [$ctrl.credential.name])"></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

tr? Should be $translate now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I need to rebase on develop branch.

@@ -0,0 +1,10 @@
angular
.module("UserAccount")
.config(["$stateProvider", function ($stateProvider) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to inject $stateProvider before the function, grunt ng-annotate task do it for you :)

@@ -0,0 +1,40 @@
angular.module("UserAccount.services")
.service("UseraccountCredentialsService", class UseraccountCredentialsService {
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be in ovh-api-services :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! I'll have to merge and release this pull request first.

}

getCredentials () {
return this.OvhHttp.get("/me/api/credential", {
Copy link
Contributor

Choose a reason for hiding this comment

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

you should use a variable for the pattern: "/me/api/credential" and use it across your methods :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OvhApiMe will be used.

</oui-column>
<oui-column data-title=":: i18n.user_credentials_desc">
<span data-ng-if="$row.description.length > 20"
data-ng-bind="($row.description | limitTo:'15') + '…'"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

this.Alerter = Alerter;
this.UseraccountCredentialsService = UseraccountCredentialsService;

this.$scope.$on("useraccount.credentials.refresh", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to $onInit ?

return this.UseraccountCredentialsService.getCredentials()
.then((credentialsIds) => {
this.credentialsIds = credentialsIds;
return credentialsIds;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed ? You already have this.credentialsIds at this point

}

transformItem (credential) {
return this.UseraccountCredentialsService
Copy link
Contributor

Choose a reason for hiding this comment

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

return this.UseraccountCredentialsService
	.getCredential(credential.id)
    .then((credentialInfo) => this.UseraccountCredentialsService
    	.getApplication(credentialInfo.credentialId)
        .then((applicationInfo) => _.assign(credentialInfo, applicationInfo)));

to

return this.UseraccountCredentialsService
	.getCredential(credential.id)
    .then((credentialInfo) => this.UseraccountCredentialsService.getApplication(credentialInfo.credentialId))
    .then((applicationInfo) => _.assign(credentialInfo, applicationInfo));

<td>
<span class="label"
data-ng-class="{
'label-danger': rule.method === 'DELETE',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is DELETE considered a "danger" ? I understand it's only a matter of color but it's a bit weird, was this CX validated ?

@@ -0,0 +1,33 @@
"use strict";
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for this if you use a class

this.$scope = $scope;
this.UseraccountCredentialsService = UseraccountCredentialsService;
this.Alerter = Alerter;
this.credential = $scope.currentActionData;
Copy link
Contributor

Choose a reason for hiding this comment

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

You can move this to $onInit

.controller("UserAccount.controllers.credentials.read", class UserAccountCredentialsReadController {
constructor ($scope) {
this.$scope = $scope;
this.credential = $scope.currentActionData;
Copy link
Contributor

Choose a reason for hiding this comment

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

You can move this to $onInit

@antleblanc
Copy link
Contributor Author

I close this PR due to inactivity. I plan to re-open it in a near future.

@antleblanc antleblanc closed this Jun 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

4 participants