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

[Monitor Type] Real Browser with Keyword & Iframe Support #4116

Closed
wants to merge 5 commits into from

Conversation

bthurlow
Copy link

@bthurlow bthurlow commented Nov 28, 2023

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Fixes #4068

Adds a real browser keyword monitoring type that has the ability to examine the content of iframes while looking for the keyword.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

image
image
image

@chakflying

This comment was marked as resolved.

@louislam louislam added the question Further information is requested label Dec 2, 2023
@chakflying chakflying added the area:monitor Everything related to monitors label Dec 2, 2023
@bthurlow

This comment was marked as resolved.

@chakflying chakflying removed the question Further information is requested label Dec 5, 2023
@chakflying chakflying changed the title [New Monitor Type] Real Browser Keyword with Iframe Support. [Monitor Type] Real Browser with Keyword & Iframe Support Dec 5, 2023
@chakflying
Copy link
Collaborator

Note that this conflicts with #3919.

@louislam louislam added the question Further information is requested label Dec 5, 2023
@louislam
Copy link
Owner

louislam commented Dec 5, 2023

And duplicate code, which make it hard to maintain in the future. If I want to fix bug of this I have to fix it 2 times.

Those new features in real-browser such as remote browser and enlarge image I believe they are not available for this due to duplicate code.

According to our pull request rules, we should have a discussion on how to design it first.

@MarcAroni81
Copy link

Hi @bthurlow, any chance you can get into discussion with Louis ? I would really like to see that feature in Uptime Kuma.

BR and thanks for your efforts
Marcus

@bthurlow
Copy link
Author

@louislam If it suits you, once I can get back on this code base, I'll see about consolidating all the keyword code into a single method to make management and reuse much better.

cc: @MarcAroni81

@deckardstp

This comment was marked as spam.

@CommanderStorm
Copy link
Collaborator

Extracting keyword and other checks is tracked in #3919
I think closing this PR is better. @chakflying do you agree?

@CommanderStorm CommanderStorm added the pr:please address review comments this PR needs a bit more work to be mergable label May 19, 2024
@github-actions github-actions bot added pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again and removed pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again labels May 19, 2024
@github-actions github-actions bot added pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again and removed pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again labels May 21, 2024
@CommanderStorm
Copy link
Collaborator

If it suits you, once I can get back on this code base, I'll see about consolidating all the keyword code into a single method to make management and reuse much better.

@bthurlow thanks for your work.
I am going to close the PR, as in the current state it is just not maintainable.
If you consolidate this into one monitor, we can reopen the PR or start of fresh in a new PR ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors needs:resolve-merge-conflict pr:please address review comments this PR needs a bit more work to be mergable pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Real Browser] Allow keyword lookup
6 participants