Skip to content

IOT-46: Time limited access #202

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

Open
wants to merge 1 commit into
base: stage
Choose a base branch
from

Conversation

fcv-iteratorIt
Copy link
Contributor

No description provided.

Copy link
Contributor

@augusthjerrild augusthjerrild left a comment

Choose a reason for hiding this comment

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

I might have misunderstood it. But if i give my user an expire date from 2 days ago, i would expect that the user could not login. Right now that is possible. It might be an issue in the backend :)

@@ -21,6 +21,10 @@ <h3>{{ "USERS.DETAIL.HEADLINE" | translate }}</h3>
<strong>{{ "USERS.DETAIL.STATUS" | translate }}</strong
>{{ user.active | activeDeactive }}
</p>
<p *ngIf="user.expiresOn">
<strong>{{ "API-KEY.EXPIRES-ON" | translate }}</strong>
{{ (user.expiresOn) | dateOnly }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be done so it doesnt always translate to english? If it's not easy then it might not matter

{{ "API-KEY.EXPIRES-ON" | translate }}
</th>
<td *matCellDef="let element" mat-cell>
{{ (element.expiresOn | dateOnly) ?? '-' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be done so it doesnt always translate to english? If it's not easy then it might not matter

@@ -21,6 +21,10 @@ <h3>{{ "USERS.DETAIL.HEADLINE" | translate }}</h3>
<strong>{{ "USERS.DETAIL.STATUS" | translate }}</strong
>{{ user.active | activeDeactive }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we miss a check here? If the user is expired this should show deactivate, shouldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Users are disabled by a service in the backend. So their expiration date is not validated expicitly in the frontend

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense :)

@@ -96,14 +97,31 @@
</div>
</div>

<div class="row form-group mb-5">
Copy link
Contributor

Choose a reason for hiding this comment

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

it's very close that we should make this as a component instead since it's used more than once almost copy pasted :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, but simple wrapper components are not that simple in Angular, and tends to add more overhead than saved. At least in my opinion

Copy link
Contributor

Choose a reason for hiding this comment

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

That is true. It should be possible to create these standalone components which requires a little less boilerplate, but agreed that this might be a small enough component.

If we need some kind of datepicker input field we could consider it later.

<div class="row form-group mb-5">
<label class="form-label">{{ "API-KEY.EDIT.EXPIRATION-DATE" | translate }}</label>
<mat-form-field appearance="outline">
<mat-label>{{ "API-KEY.EDIT.EXPIRATION-DATE-PLACEHOLDER" | translate }}</mat-label>
Copy link
Contributor

Choose a reason for hiding this comment

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

guess it's the same but the api-key might be misleading since this is user. Argument of why we should consider making a shared component

<div class="row mb-5">
<mat-slide-toggle [(ngModel)]="user.active" id="active" name="active">
{{ "USERS.FORM.ACTIVE" | translate }}</mat-slide-toggle
{{ "USERS.FORM.ACTIVE" | translate }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be disabled if the user is expired?

{{ "USERS.STATUS" | translate }}
</th>
<td mat-cell *matCellDef="let element">
<td *matCellDef="let element" mat-cell>
<p [ngClass]="element.active | activeDeactive">{{ element.active | activeDeactive }}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Like in the api key, should't this show deactivated if the user is expired?

@@ -1315,12 +1316,17 @@
"NAME-PLACEHOLDER": "Indtast nøglens navn",
"CANCEL": "Annuller",
"SAVE": "Gem nøgle",
"CREATE-API-KEY": "Opret nøgle"
"CREATE-API-KEY": "Opret nøgle",
"EXPIRATION-DATE": "Udløbsdato",
Copy link
Contributor

Choose a reason for hiding this comment

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

in the jira task the word "Deaktiveringsdato" is used instead. Any reason for using udløbsdato?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants