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.

Sign up for GitHub

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

Jump to bottom

Goto site step in the browser steps doesn't work even though it is part of the steps. #2330

Closed
manojVivek opened this issue Apr 24, 2024 · 8 comments
Closed

Goto site step in the browser steps doesn't work even though it is part of the steps. #2330

manojVivek opened this issue Apr 24, 2024 · 8 comments
Assignees
@dgtlmoon
Labels
browser-steps bug Something isn't working

Comments

@manojVivek
Copy link

Describe the bug
Any Goto site step that is at step index > 0 of the browser steps is not getting executed and getting skipped silently.

Version
v0.45.20

To Reproduce

Steps to reproduce the behavior:
Create a browser steps workflow with the following steps:
Step 1: Goto site
Step 2: Click element containing text "Login"
Step 3: Enter text in field ".username" "some-username"
Step 4: Enter text in field ".password" "some-password"
Step 5: Click element button.login
Step 6: Goto site
Step 7: Click element containing text "show data"

Now, in this example, the step 6 is always skipped when the run is performed, it goes to step 7 after step 5.

! ALWAYS INCLUDE AN EXAMPLE URL WHERE IT IS POSSIBLE TO RE-CREATE THE ISSUE - USE THE 'SHARE WATCH' FEATURE AND PASTE IN THE SHARE-LINK!

Expected behavior
It runs the Goto site that is part of the browser steps

Additional context
I went through the code a bit and found that the Goto site step is being filtered out in the browser_steps_get_valid_steps function here:

changedetection.io/changedetectionio/content_fetchers/base.py

Line 116 in 1ba2965

lambda s: (s['operation'] and len(s['operation']) and s['operation'] != 'Choose one' and s['operation'] != 'Goto site'),

It makes sense to just remove the Goto site step when it is at Step 1 of the steps, but it should not be filtered when it is at a step other than 1.

Please let me know if you have any questions.
@dgtlmoon
Copy link
Owner

fantastic report, thanks! will check it out

@dgtlmoon dgtlmoon added bug Something isn't working browser-steps and removed triage labels Apr 24, 2024
@manojVivek
Copy link
Author

@dgtlmoon If you are fine with the following fix, I can try and make a PR if you are fine with the following fix:

  1. Filter out the Goto site step only when it is at Step 1.
  2. Swap any other Goto site action to Goto URL, watch.url.

Please let me know your thoughts.

@dgtlmoon
Copy link
Owner

@dgtlmoon If you are fine with the following fix, I can try and make a PR if you are fine with the following fix:

  1. Filter out the Goto site step only when it is at Step 1.
  2. Swap any other Goto site action to Goto URL, watch.url.

Please let me know your thoughts.

yeah goto site should be the first step always

Goto URL should be every other step

problem is you'll also need to create an update hook so that you dont break everyones installations if you simply rename the step...

@manojVivek
Copy link
Author

problem is you'll also need to create an update hook so that you dont break everyones installations if you simply rename the step...

No, I didn't mean to replace all Goto site to Goto URL in the datastore. All other Goto site will still be the same in the datastore.

But during the execution, we can do an inline conversion of all non-first-step Goto site into Goto URL with watch.url as arg like in

changedetection.io/changedetectionio/blueprint/browser_steps/__init__.py

Lines 170 to 173 in ee52947

if step_operation == 'Goto site':
step_operation = 'goto_url'
step_optional_value = datastore.data['watching'][uuid].get('url')
step_selector = None

Wdyt?

@manojVivek manojVivek mentioned this issue Apr 27, 2024
Closed
@manojVivek
Copy link
Author

I have implemented the above explained approach in this PR #2337, and it worked well for the use case that was failing for me before.

@dgtlmoon
Copy link
Owner

Create a browser steps workflow with the following steps:
Step 1: Goto site
Step 2: Click element containing text "Login"
Step 3: Enter text in field ".username" "some-username"
Step 4: Enter text in field ".password" "some-password"
Step 5: Click element button.login
Step 6: Goto site
Step 7: Click element containing text "show data"

why are you even using "goto site" as the first step?

dgtlmoon added a commit that referenced this issue May 14, 2024
@dgtlmoon
Closes #2330 #2337
a9e7002
dgtlmoon added a commit that referenced this issue May 15, 2024
@dgtlmoon
Browser Steps - Fixing "goto site" step #2330 #2337 (#2364)
e4504fe
@manojVivek
Copy link
Author

Create a browser steps workflow with the following steps: Step 1: Goto site Step 2: Click element containing text "Login" Step 3: Enter text in field ".username" "some-username" Step 4: Enter text in field ".password" "some-password" Step 5: Click element button.login Step 6: Goto site Step 7: Click element containing text "show data"

why are you even using "goto site" as the first step?

I'm not using that, I referred this default "Goto Site" step:
Screenshot 2024-05-15 at 2 21 03 PM

@dgtlmoon
Copy link
Owner

should be resolved with the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dgtlmoon dgtlmoon

Labels
browser-steps bug Something isn't working
Projects
None yet
Milestone
No milestone
2 participants
@dgtlmoon @manojVivek