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

Added browser storage handling as seperate extension #1025

Merged
merged 6 commits into from
Dec 9, 2021
Merged

Added browser storage handling as seperate extension #1025

merged 6 commits into from
Dec 9, 2021

Conversation

kevinkammleiter
Copy link
Contributor

Hello guys,
we have a little contribution for you and it would be great, if you could review the changes and hopefully bringing this in a future release ☺️

  • For authentication purposes it could be useful, to have the ability to inject several session-/localStorage values into the page handled by puppeteer.

  • This extension will use the phantomas parameter sessionStorage and localStorage and inject them into the page storage respectively.

@macbre
Copy link
Owner

macbre commented Nov 17, 2021

Thanks for your PR, @kevinkammleiter !

A few small things to clean up before this can be merged:

  1. Please update bin/program.js to include options that you've introduced.
  2. Run prettier for the code you've added.
  3. The code you've added is not tested, hence jest complains about the code coverage going below the threshold. Please consider adding tests. Cookies handling tests should be good guideline here.

Thanks once again!

@macbre macbre added this to the v2.4 milestone Nov 17, 2021
@kevinkammleiter
Copy link
Contributor Author

Thanks for your PR, @kevinkammleiter !

A few small things to clean up before this can be merged:

  1. Please update bin/program.js to include options that you've introduced.
  2. Run prettier for the code you've added.
  3. The code you've added is not tested, hence jest complains about the code coverage going below the threshold. Please consider adding tests. Cookies handling tests should be good guideline here.

Thanks once again!

Thanks for the suggested improvements. 👍

I will start soon with implementing the changes and update the pull request.

@kevinkammleiter
Copy link
Contributor Author

Hi @macbre!

I added the changes you suggested.

Thanks again for the hints! 👍

bin/program.js Outdated Show resolved Hide resolved
"session-storage": "bar=foo"
metrics:
session-storage: '{"bar":"foo"}'

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks!

@macbre
Copy link
Owner

macbre commented Dec 7, 2021

@kevinkammleiter - thanks for contributing, merging!

@macbre
Copy link
Owner

macbre commented Dec 8, 2021

Actually, @kevinkammleiter - can you apply my above suggestion (regarding wording in option) and merge the latest devel branch changes to your fork / branch?

@macbre macbre enabled auto-merge (squash) December 9, 2021 08:07
@macbre macbre merged commit bab0e54 into macbre:devel Dec 9, 2021
@macbre
Copy link
Owner

macbre commented Jan 7, 2022

@kevinkammleiter - published in https://github.com/macbre/phantomas/releases/tag/v2.4.0

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

Successfully merging this pull request may close these issues.

None yet

2 participants