Skip to content

fix: adds the missing expire attribute to ListSessions call#4872

Closed
CoderMayhem wants to merge 2 commits into
appwrite:masterfrom
CoderMayhem:fix-4846-adding-expire-to-send-in-ListSessions
Closed

fix: adds the missing expire attribute to ListSessions call#4872
CoderMayhem wants to merge 2 commits into
appwrite:masterfrom
CoderMayhem:fix-4846-adding-expire-to-send-in-ListSessions

Conversation

@CoderMayhem

@CoderMayhem CoderMayhem commented Dec 16, 2022

Copy link
Copy Markdown

What does this PR do?

Adds the missing expire attribute to the ListSessions response. Linked to issue #4846

Test Plan

Changes were done as suggested in the issue discussion thread by @stnguyen90. I couldn't figure out a way to test it and would appreciate any help in setting up a dummy call to the API and check for ListSessions output.

Related PRs and Issues

Fixes #4846

Have you added your change to the Changelog?

Yes

(The CHANGES.md file tracks all the changes that make it to the main branch. Add your change to this file in the following format)

  • One line description of your PR [#pr_number](Link to your PR)

Have you read the Contributing Guidelines on issues?

Yes


$session->setAttribute('countryName', $countryName);
$session->setAttribute('current', ($current == $session->getId()) ? true : false);
$session->setAttribute('expire', DateTime::addSeconds(new \DateTime($session->getCreatedAt()), $authDuration));

@stnguyen90 stnguyen90 Jan 10, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The value should be formatted like an ISO string as well.

@stnguyen90 stnguyen90 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great PR! 🤯 We left some comments during the review, please check them out.

@stnguyen90

Copy link
Copy Markdown
Contributor

@CoderMayhem are you still able to work on this PR or should this be reassigned?

@CoderMayhem

Copy link
Copy Markdown
Author

Hey @stnguyen90, looks like I missed your comment. I apologize. I'll do the changes requested and push them.

@stnguyen90

Copy link
Copy Markdown
Contributor

@CoderMayhem, how's your progress on this? Also, it looks like there's a merge conflict now.

@stnguyen90

Copy link
Copy Markdown
Contributor

@CoderMayhem, FYI, I'll need to close this due to inactivity soon.

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.

🐛 Bug Report: expire is empty from account.listSessions()

2 participants