-
Notifications
You must be signed in to change notification settings - Fork 42
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
Login and Logout handler #3366
base: main
Are you sure you want to change the base?
Login and Logout handler #3366
Conversation
…d to undo that embarrassment
…ed passing unit tests
Someone is attempting to deploy a commit to the Inrupt Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @zg009, and thanks so much for contributing! Do not worry about the end-to-end tests: once the code is in a stable state, I'll rebase your commits from your fork to a branch on the main repository, which will run all the tests. I added a couple of comments on the PR, and you can also add a changelog entry if you wish, otherwise I'll add it when rebasing. In any case, you'll be credited as a contributor to the code in the commit history, and I'll mention you in the changelog entry :)
…r packages, renamed LOGIN_AND_LOGOUT to SESSION_STATUS_CHANGE
I do not know how to add a changelog entry, sorry. I am also unsure what configs are conflicting in the e2e-test file |
New feature description
Addresses #422
The simplest approach was to add an entirely new EVENTS const which is simply emitted at all locations along with the login and logout existing events. I was unable to run the E2E tests, I figure it was due to the confusion on provisioning the pod base uri. I added unit tests which passed under Jest, and conformed to the existing test structure. The commit messages could be improved upon. I can create a new fork if needed.
Checklist
index.ts
, if applicable.