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

Handling the goto site steps which were previously skipped in the browser steps processing #2337

Conversation

manojVivek
Copy link

Fixes #2330


return valid_steps

return None

def iterate_browser_steps(self):
def iterate_browser_steps(self, url):
Copy link
Owner

Choose a reason for hiding this comment

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

seems confusing and vague why this has url here

can you rename it to something like start_url ? or? im confused

Copy link
Author

Choose a reason for hiding this comment

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

Done.

# If the operation is Goto site, change it to Goto URL and use the url as the optional value
if step['operation'] == 'Goto site':
step['operation'] = 'Goto URL'
optional_value = url
Copy link
Owner

Choose a reason for hiding this comment

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

the function has url but its only used here? seems weird

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@@ -172,7 +172,7 @@ def run(self,

# Run Browser Steps here
if self.browser_steps_get_valid_steps():
self.iterate_browser_steps()
self.iterate_browser_steps(url)
Copy link
Owner

Choose a reason for hiding this comment

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

try to use start_url = url or something

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@manojVivek manojVivek requested a review from dgtlmoon May 12, 2024 04:46
@dgtlmoon
Copy link
Owner

image

this comment is wrong, the first step is actually just empty

dgtlmoon added a commit that referenced this pull request May 14, 2024
@manojVivek
Copy link
Author

It looks like you pushed a related commit, do I need to do anything on top of this?

@dgtlmoon
Copy link
Owner

It looks like you pushed a related commit, do I need to do anything on top of this?

Nope all good, i ended up going it my own way because there was quite a few other changes/improvements , thanks for the inspiration!

@dgtlmoon dgtlmoon closed this May 15, 2024
@manojVivek
Copy link
Author

Thanks for taking care of it! ❤️

@dgtlmoon
Copy link
Owner

@manojVivek any chance you can pull and test the dev container tag when https://github.com/dgtlmoon/changedetection.io/actions/runs/9092745536 is finished?

@manojVivek
Copy link
Author

manojVivek commented May 15, 2024

@dgtlmoon Tested and works great for me. 👏🏼

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.

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