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

DBP: adding other actions #844

Merged
merged 7 commits into from
Nov 22, 2023
Merged

DBP: adding other actions #844

merged 7 commits into from
Nov 22, 2023

Conversation

shakyShane
Copy link
Collaborator

@shakyShane shakyShane commented Nov 21, 2023

https://app.asana.com/0/0/1206000767975136/f

Feature Summary

Native platforms will send one of the following instructions, one at a time:

  • navigate
  • extract
  • click <- already added in a previous PR
  • expectation
  • fillForm
  • getCaptchaInfo
  • solveCaptcha

Each of them has a corresponding function that will be executed, and will return either a SuccessResponse or ErrorResponse to native.

The flow looks like this: (taken from the markdown file in the PR)

image

@shakyShane shakyShane mentioned this pull request Nov 21, 2023
@shakyShane
Copy link
Collaborator Author

shakyShane commented Nov 21, 2023

@shakyShane shakyShane mentioned this pull request Nov 21, 2023
@shakyShane shakyShane changed the title adding other actions DBP: adding other actions Nov 21, 2023
@shakyShane shakyShane marked this pull request as ready for review November 21, 2023 14:36
@shakyShane shakyShane assigned shakyShane and muodov and unassigned shakyShane Nov 21, 2023
@shakyShane
Copy link
Collaborator Author

@muodov this adds all the other actions, the next PR has all the tests.

Please let me know if you'd like me to break this down further, I'd be happy to do so if it's easier to review

brianhall
brianhall previously approved these changes Nov 21, 2023
Copy link
Contributor

@brianhall brianhall left a comment

Choose a reason for hiding this comment

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

This LGTM, no blockers, but a few questions/comments.

src/features/broker-protection/buildUrl.js Outdated Show resolved Hide resolved
src/features/broker-protection/comparison-functions.js Outdated Show resolved Hide resolved
src/features/broker-protection/comparison-functions.js Outdated Show resolved Hide resolved
src/features/broker-protection/fillForm.js Outdated Show resolved Hide resolved
Base automatically changed from 11-21-first_action to main November 22, 2023 07:50
@shakyShane shakyShane dismissed brianhall’s stale review November 22, 2023 07:50

The base branch was changed.

package.json Outdated Show resolved Hide resolved
@shakyShane
Copy link
Collaborator Author

@jonathanKingston @muodov - I've added a few extra commits to address the feedback, thanks!

I think we'll ready to merge this, and then it's onto the integration tests :)

@brianhall
Copy link
Contributor

@shakyShane (Adding this from mobile, so apologies if I’m not seeing this change and for the late comment). We spoke on Asana about adding playwright as a top level dependency, can we do that in this PR?

@shakyShane
Copy link
Collaborator Author

@brianhall we can do that in the next one, which is 100% playwright tests :)

Copy link
Member

@muodov muodov left a comment

Choose a reason for hiding this comment

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

looks good, just one remark about url encoding. Thanks for the changes!

@shakyShane
Copy link
Collaborator Author

Thanks! merging now since we have approvals from @brianhall and @muodov, I also addressed feedback from @jonathanKingston

@shakyShane shakyShane merged commit 3b1a924 into main Nov 22, 2023
8 checks passed
@shakyShane shakyShane deleted the 11-21-adding_other_actions branch November 22, 2023 15:12
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.

None yet

4 participants