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

feat: remove deprecated Evented API usage from the public session service #2526

Conversation

BobrImperator
Copy link
Collaborator

@BobrImperator BobrImperator commented Mar 23, 2023

  • Removes deprecated Evented API usage from the public session service.
    handleAuthentication and handleInvalidation methods should be used instead.

Example usage of handleAuthentication https://github.com/mainmatter/ember-simple-auth/blob/master/guides/managing-current-user.md

@BobrImperator BobrImperator force-pushed the feat-remove-evented-api-from-session branch from 5bb32dc to 698fa2f Compare March 23, 2023 17:01
@BobrImperator BobrImperator merged commit 6b3f57e into mainmatter:master Mar 24, 2023
@BobrImperator BobrImperator deleted the feat-remove-evented-api-from-session branch March 24, 2023 10:43
@RobbieTheWagner
Copy link
Contributor

I'm not sure what I am doing wrong, but handleAuthentication never gets called for me. I used to use this.session.on('authenticationSucceeded', which worked great, but now I have nothing that works.

@BobrImperator
Copy link
Collaborator Author

@RobbieTheWagner
Copy link
Contributor

I am calling the setup method and have been for a long time, so it's not that.

@BobrImperator
Copy link
Collaborator Author

BobrImperator commented Jul 29, 2023

Other scenario could be misplaced handleAuthentication method.
I remember it used to exist for an application route at some point but was removed with mixins and the service should be extended instead.

I don't see how else it could break other than the service not existing at all.

You could inspect the classic-test-app and test-app to see how to use session service now. This PR specifically that removes mixins https://github.com/mainmatter/ember-simple-auth/pull/2480/files

The event logic was also removed from the public service only while the internal one still has them and they are actually being used for triggering the handleAuthentication method.
We didn't change much in terms of internal logic so far, just the public interface.

Technically you could do this.session.session.on('authenticationSucceeded') and it should work in the same way.

It's hard to talk about this without any code, but everything should work the same as long as mixins were correctly replaced with the session methods.

If there are no events being sent then it could be that an authenticator is failing, but in that case you should at least see something logged to the console.

@RobbieTheWagner
Copy link
Contributor

@BobrImperator this is the PR updating things. It has a failing test because handleAuthentication is never called shipshapecode/swach#1732

We do use https://github.com/adopted-ember-addons/ember-cognito and I am not sure if it needs changes to work with latest?

@BobrImperator
Copy link
Collaborator Author

I made a PR for Swach, please see: shipshapecode/swach#1736

@hoIIer
Copy link

hoIIer commented Sep 1, 2024

Hey @BobrImperator, upgrading an ember app and hit session.on not existing.

What is the correct way to have a component re-render upon app authentication now?

Also the event api still in the docs?

https://ember-simple-auth.com/api/SessionService.html#.event:authenticationSucceeded

@BobrImperator
Copy link
Collaborator Author

BobrImperator commented Sep 2, 2024

Hi @hoIIer

The Evented API had been removed from the service, a direct replacement would be extending the service and implementing a handleAuthentication and handleInvalidation.

e.g. Test app example

import { inject as service } from '@ember/service';

import Session from 'ember-simple-auth/services/session';

export default class SessionService extends Session {

  handleAuthentication() {
    super.handleAuthentication(...arguments);
    
    // Do you thing here
  }
}

Although if your only requirement is to render a specific thing on authenticated session, then you should be able to use the service's isAuthenticated property.

{{!-- app/templates/application.hbs --}}
<div class="menu">
  …
  {{#if this.session.isAuthenticated}}
    <a {{on "click" this.invalidateSession}}>Logout</a>
  {{else}}
    {{#link-to 'login'}}Login{{/link-to}}
  {{/if}}
</div>
<div class="main">
  {{outlet}}
</div>

See Walkthrough and Other guides in the readme

Lastly

Also the event api still in the docs?

That appears to be a mistake, since the public SessionService doesn't emit anything and only hooks itself up to the internal, private one.
We'll need to clean this up

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

Successfully merging this pull request may close these issues.

4 participants