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

📚 Documentation: The documentation/method Account.deleteSession() and Account.deleteSessions() are wrong #153

Closed
2 tasks done
qiqetes opened this issue Jun 7, 2023 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@qiqetes
Copy link

qiqetes commented Jun 7, 2023

💭 Description

I'll paste the source and you will notice the error:

  /// Delete Session
  ///
  /// Use this endpoint to log out the currently logged in user from all their
  /// account sessions across all of their different devices. When using the
  /// Session ID argument, only the unique session ID provided is deleted.
  ///
  ///
  Future deleteSession({required String sessionId}) async {
    final String path =
        '/account/sessions/{sessionId}'.replaceAll('{sessionId}', sessionId);

    final Map<String, dynamic> params = {};

    final Map<String, String> headers = {
      'content-type': 'application/json',
    };

    final res = await client.call(HttpMethod.delete,
        path: path, params: params, headers: headers);

    return res.data;
  }
  
  /// Delete Sessions
  ///
  /// Delete all sessions from the user account and remove any sessions cookies
  /// from the end client.
  ///
  Future deleteSessions() async {
    const String path = '/account/sessions';

    final Map<String, dynamic> params = {};

    final Map<String, String> headers = {
      'content-type': 'application/json',
    };

    final res = await client.call(HttpMethod.delete,
        path: path, params: params, headers: headers);

    return res.data;
  }
  1. The param sessionId is required, so it will be only one session expired.
  2. I don't see that passing 'current' as sessionId will expire the current session, would be nice since in the method getSession() works like that.
  3. Seems like the function that does log out the currently logged in user from all their account sessions across all of their different devices is the deleteSessions.
  4. Does it really remove any sessions cookies?

Proposed

Remove the method deleteSessions, and make the sessionId parameter optional. Change the description so it includes 'current' if it really works like that.
If you tell me how to act I can provide the PR

👀 Have you spent some time to check if this issue has been raised before?

  • I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

@qiqetes qiqetes added the documentation Improvements or additions to documentation label Jun 7, 2023
@lohanidamodar
Copy link
Member

@qiqetes Thank you for raising the issue.

@gewenyu99
Copy link

This is purely a documentation issue across all SDKs. The behavior is correct, the docs are out of whack. Thanks for letting us know!

@stnguyen90
Copy link
Contributor

Closing as this is just pending deployment.

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

No branches or pull requests

4 participants