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.

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

modernize addon: moved from webuniversum to appuniversum, converted to glimmer components #4

Merged
merged 13 commits into from Aug 30, 2021
Merged
6 changes: 6 additions & 0 deletions addon/components/acmidm-login.hbs
@@ -0,0 +1,6 @@
<AuButton @loading={{this.isAuthenticating}} @disabled={{this.isAuthenticating}} {{on "click" this.login}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using (and depending on) Appuniversum, it might be nice to keep these components styling agnostic?

I think this can easily be achieved by yielding state instead of outputting html. That way projects can decide which components to use.

Depending on appuniversum will also create an extra maintenance burden since it will need to be bumped from time to time (especially since it's a 0.x release).

Copy link
Contributor

@Windvis Windvis Aug 9, 2021

Choose a reason for hiding this comment

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

Most projects also seem to implement a "compact" version (which duplicates the logic) of this component for the main header. That would then no longer be needed either.

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 think that's a great idea, but would keep this for backwards compatibility. perhaps adding a deprecation warning in the constructor?

Copy link
Member

Choose a reason for hiding this comment

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

For the data-table we took the approach of a styling agnostic addon and another addon that extends it with Appuniversum, but that results in a heavier maintentance burden I must admit (and still makes it a breaking change for this addon atm).

I'm pro the deprecation warning in this PR. It paves the way to a styling agnostic version and thus a breaking release in the future. But I would implement that in a separate PR and not lump it on this one.

Copy link
Member

Choose a reason for hiding this comment

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

I only see now that there is a PR #5 already 🙈

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I created a follow-up PR as a quick POC of what a styling agnostic version could look like. I don't think the deprecation warnings should be added here since there are no alternatives yet, but I can add them to that PR as well.

Meld u aan
</AuButton>
{{#if this.errorMessage}}
<AuAlert @alertIcon="alert-triangle" @alertTitle={{this.errorMessage}} />
{{/if}}
33 changes: 18 additions & 15 deletions addon/components/acmidm-login.js
@@ -1,26 +1,29 @@
import { warn } from '@ember/debug';
import Component from '@ember/component';
import layout from '../templates/components/acmidm-login';
import Component from '@glimmer/component';
import { inject as service } from '@ember/service';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';

export default Component.extend({
layout,
session: service('session'),
actions: {
login() {
this.set('errorMessage', '');
this.set('isAuthenticating', true);
this.session.authenticate('authenticator:torii', 'acmidm-oauth2').catch((reason) => {
export default class AcmIdmLoginComponent extends Component {
@tracked errorMessage = "";
@tracked isAuthenticating = false;
@service session;

@action
login() {
this.errorMessage = "";
this.isAuthenticating = true;
this.session.authenticate('authenticator:torii', 'acmidm-oauth2')
.catch(reason => {
warn(reason.error || reason, { id: 'authentication.failure' });

if (reason.status == 403)
this.set('errorMessage', 'U heeft geen toegang tot deze applicatie.');
this.errorMessage = 'U heeft geen toegang tot deze applicatie.';
else
this.set('errorMessage', 'Fout bij het aanmelden. Gelieve opnieuw te proberen.');
this.errorMessage = 'Fout bij het aanmelden. Gelieve opnieuw te proberen.';
})
.finally(() => {
this.set('isAuthenticating', false);
this.isAuthenticating = false;
});
}
}
});
}
@@ -1,5 +1,7 @@
<button ...attributes {{on "click" this.logout}}>
Copy link
Contributor

@Windvis Windvis Aug 9, 2021

Choose a reason for hiding this comment

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

not sure if this component is really needed since it's just a button wrapper for ember-simple-auth's session invalidation logic (which is not really related to acm/idm either)?

Copy link
Contributor

Choose a reason for hiding this comment

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

(I just wanted to get the conversation started. If this is a wanted change we can deprecate the component first in a separate PR)

{{#if (has-block)}}
{{yield}}
{{else}}
Meld u af
{{/if}}
</button>
16 changes: 8 additions & 8 deletions addon/components/acmidm-logout.js
@@ -1,12 +1,12 @@
import Component from '@ember/component';
import layout from '../templates/components/acmidm-logout';
import Component from '@glimmer/component';
import { inject as service } from '@ember/service';
import { action } from '@ember/object';

export default Component.extend({
layout,
tagName: 'button',
session: service('session'),
click() {
export default class AcmIdmLogoutComponent extends Component {
@service session;

@action
logout() {
this.session.invalidate();
}
});
}
7 changes: 7 additions & 0 deletions addon/components/acmidm-switch.hbs
@@ -0,0 +1,7 @@
<button ...attributes {{on "click" this.switch}} disabled={{this.disabled}}>
{{#if (has-block)}}
{{yield}}
{{else}}
Wissel van bestuurseenheid
{{/if}}
</button>
43 changes: 26 additions & 17 deletions addon/components/acmidm-switch.js
@@ -1,25 +1,34 @@
import Component from '@ember/component';
import layout from '../templates/components/acmidm-switch';
import { configurable } from 'torii/configuration';
import Component from '@glimmer/component';
import { getOwner } from '@ember/application';
import fetch from 'fetch';
import {action} from '@ember/object';
import {tracked} from '@glimmer/tracking';
export default class AcmIdmSwitchComponent extends Component {
@tracked disabled = false;
constructor() {
super(...arguments);
const config = getOwner(this).resolveRegistration('config:environment')?.torii;
const providerConfig = config?.providers ? config.providers["acmidm-oauth2"] : null;
if (!providerConfig) {
throw "could not find acmidm-oauth2 configuration, make sure it is set in your application environment";
}
this.logoutUrl = providerConfig.logoutUrl;
this.clientId = providerConfig.clientId;
this.returnUrl = providerConfig.returnUrl;
}

export default Component.extend({
layout,
name: Object.freeze('acmidm-oauth2'), // used by configurable
logoutUrl: configurable('logoutUrl'),
clientId: configurable('apiKey'),
returnUrl: configurable('switchUrl'),
attributeBindings: ['disabled'],
init() {
this._super(...arguments);
this.set('disabled', false);
},
async click() {
this.set('disabled', true);
@action
async switch() {
this.disabled = true;
try {
await fetch('/sessions/current', {
method: 'DELETE',
credentials: 'same-origin'
});
window.location.replace(`${this.logoutUrl}?switch=true&client_id=${this.clientId}&post_logout_redirect_uri=${encodeURIComponent(this.returnUrl)}`);
}
catch(e) {
this.disabled = false;
}
}
});
}
7 changes: 0 additions & 7 deletions addon/templates/components/acmidm-login.hbs

This file was deleted.

5 changes: 0 additions & 5 deletions addon/templates/components/acmidm-switch.hbs

This file was deleted.

10 changes: 7 additions & 3 deletions package.json
Expand Up @@ -29,15 +29,19 @@
"ember-cli-babel": "^7.26.6",
"ember-cli-htmlbars": "^5.7.1",
"ember-simple-auth": "^3.0.0",
"@glimmer/component": "^1.0.4",
"@glimmer/tracking": "^1.0.4",
"ember-fetch": "^8.1.0",
"torii": "^0.10.1"
},
"peerDependencies": {
"@appuniversum/ember-appuniversum": ">= 0.4.0"
},
"devDependencies": {
"@appuniversum/ember-appuniversum": "^0.4.8",
"@ember/optional-features": "^2.0.0",
"@ember/test-helpers": "^2.2.5",
"@embroider/test-setup": "^0.41.0",
"@glimmer/component": "^1.0.4",
"@glimmer/tracking": "^1.0.4",
"@lblod/ember-vo-webuniversum": "^0.19.2",
"babel-eslint": "^10.1.0",
"broccoli-asset-rev": "^3.0.0",
"ember-auto-import": "^1.11.3",
Expand Down
8 changes: 7 additions & 1 deletion tests/dummy/app/templates/application.hbs
Expand Up @@ -2,4 +2,10 @@

<h2 id="title">Welcome to Ember</h2>

{{outlet}}
{{outlet}}

<AcmidmSwitch />

<AcmidmLogin />

<AcmidmLogout />
13 changes: 12 additions & 1 deletion tests/dummy/config/environment.js
Expand Up @@ -16,7 +16,18 @@ module.exports = function (environment) {
Date: false,
},
},

torii: {
disableRedirectInitializer: true,
providers: {
'acmidm-oauth2': {
apiKey: "{{OAUTH_API_KEY}}",
baseUrl: "{{OAUTH_BASE_URL}}",
scope: 'openid rrn vo profile abb_gelinktNotuleren',
redirectUri: "{{OAUTH_REDIRECT_URL}}",
logoutUrl: "{{OAUTH_LOGOUT_URL}}"
}
}
},
APP: {
// Here you can pass flags/options to your application instance
// when it is created
Expand Down