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

Add option to sync watch filters/triggers to other watches with matching watch groups #590

Closed
wants to merge 10 commits into from

Conversation

timlod
Copy link
Contributor

@timlod timlod commented May 8, 2022

Same, but slightly updated, content as #505, merging onto master with suggested changes:

This PR adds a checkbox on the filters/triggers watch edit page which, when ticked, copies the settings from that page to all other watches with the same watch groups.

Motivation:
Someone may watch several subpages of the same domain, which share some boilerplate HTML. When some text changes in the boilerplate, all those watches will exhibit the same change. To prevent having to edit all those pages one by one, with this setting you can now filter/remove those elements for all those watches at once (given that those pages share the same tags).

For the test, I had to add an API parameter /edit/last, which returns the last watch. The test adds 3 pages, the first and last of which share tags. The test then shows that the changed setting is copied over only to the last, but not the second, watch.

@dgtlmoon
Copy link
Owner

dgtlmoon commented May 13, 2022

would it be possible to have a [groups] tab in the main settings that

  • shows a list of groups
  • shows which groups are being synced
  • offers ability to turn off syncing for those group

just a list with a checkbox before each name would be fine i think

image

This for me feels good because I know I can find all the settings in one place, aswell as on a per-watch level

it also helps me to know what the 'state' of the configuration is in

@timlod
Copy link
Contributor Author

timlod commented May 14, 2022

would it be possible to have a [groups] tab in the main settings that

* shows a list of groups

* shows which groups are being synced

* offers ability to turn off syncing for those group

just a list with a checkbox before each name would be fine i think

image

This for me feels good because I know I can find all the settings in one place, aswell as on a per-watch level

it also helps me to know what the 'state' of the configuration is in

I think this would be nice, but I see some issues that need some thought:

  1. does syncing settings from this overview mean that all watches get a "union" of all the settings already inside the watch group?
  2. do future edits to a single page automatically sync to all other watches with matching watch groups? (I assume yes)
  3. is it possible at all to have single watches have more specific settings if syncing is toggled (without sharing)?

I'm not sure what I think is the best answer for these - the easiest to understand is certainly that everything is always shared, and that to modify one would have to toggle the setting off, and then make changes. What do you think?

In essence, this will also be a different feature to syncing results vs. the current implementation. To be clear, the way the toggle on the setting page currently works is that it syncs settings only to watches that matches ALL watch groups, i.e:

  • example1.com has tags "a, b, c"
  • example2.com has tags "b, c"
  • example3.com has tags "a, b, c"
  • example4.com has tags "a"

If you sync settings from example1, it would only copy them over only to example3. If you sync settings from example2, it would copy them to example1 and 3. If you sync settings from example4, it would copy them to example1 and 3 as well. This is to allow using this without sharing settings to more general watch groups.

So this new feature would allow toggles on a, b and c directly - I actually like that it gives you another way to copy settings, but it needs to be made clear how it resolves existing settings across watches with that tag. I'll start implementing the basics of that feature, but I think it's sufficiently different to have that in a separate PR - would you agree?

@dgtlmoon
Copy link
Owner

One way my bank website deals with this, is that it lets you

"[ ] save this as a template for future use"

then the future ones has a select list of ones that are marked as an existing template

sort of like "Copy filter/trigger settings to all watches with matching watch groups')" but more passive i guess

what do you think?

@dgtlmoon
Copy link
Owner

"Copy filter/trigger settings to all watches with matching watch groups')"

I think this is like.. you set it up, and then you push it onto all the other group right?

You still cant actually re-use it by selecting an existing one (like my template comment above)

@timlod
Copy link
Contributor Author

timlod commented May 15, 2022

"Copy filter/trigger settings to all watches with matching watch groups')"

I think this is like.. you set it up, and then you push it onto all the other group right?

You still cant actually re-use it by selecting an existing one (like my template comment above)

This is correct - mainly because in the original intended use filters would be very specific to a watch group, and wouldn't need to be reused for other sites. However, I see how saving settings as templates adds a lot of flexibility that could be useful.

Hence:

One way my bank website deals with this, is that it lets you

"[ ] save this as a template for future use"

then the future ones has a select list of ones that are marked as an existing template

sort of like "Copy filter/trigger settings to all watches with matching watch groups')" but more passive i guess

what do you think?

I like this, but think this would then be a third feature. So far, I count three distinct features as to sharing/reusing settings (across watch groups):

  1. This PR - push settings to other watches sharing all watch groups
  2. Watch group sync overview in global settings, allowing to toggle sharing of settings
  3. Another toggle in the edit page to save settings as template, including (presumably) a template dropdown to take settings from existing templates, as well as a template overview in the global settings (to modify and delete templates)

In principle, templates could be used to do what I implemented here (1.), but in our specific case would lead to way more config needed to set up, as we'd need to select the template for each watch (our case is hundreds of watches, with up to around 20 links in a single watch group). It would be less if we combined it with (2.), however, we'd still have to toggle all watch groups, and we're suddenly constrained to having the exact same settings within watch groups, unless there is another feature to exclude settings from syncing (adding a lot of complexity).

In general, I prefer (1.) over (2.), as it'll always be a conscious choice to push settings to other watches, whereas with passive syncing I'd see it as more likely that settings get overwritten unintentionally.

I'm a bit unsure now where to go from here, as there are a lot of potential implementations which all have a somewhat different UX.

@dgtlmoon
Copy link
Owner

I'm thinking whats going to be cleanest for the rest of the people who use this app

maybe the solution is actually to setup some kind of settings for each group that contain a template?

Do you have basically all of the required 'settings' tied cleanly to each 'group' ?

@timlod
Copy link
Contributor Author

timlod commented May 18, 2022

I'm also wondering that. What do you mean by this?:

maybe the solution is actually to setup some kind of settings for each group that contain a template?

Do you have basically all of the required 'settings' tied cleanly to each 'group' ?

I still think that settings may often be shared among the group, but potentially with extras for each individual watch. That's why I think just syncing probably isn't going to be ideal, as flexibility is lost. What's best for all users, I can't be sure of - there will probably be lots of edge cases depending on how people have set up their watch groups.

What do you think about adding some kind of drop-down in this PR which let's you choose which watch group(s) to push settings to (ie. full group match, or single shared group)? For any such operations then the extra consideration is whether to overwrite previous settings, or add on top of them.

@dgtlmoon
Copy link
Owner

dgtlmoon commented May 18, 2022

So you want maybe a template/setting for each group, but then once you edit an individual watch, those settings stick to that watch only, right? - AND you want to be able to say "Ok, time to update ALL of these in the group back to the template, because everything changed", which simply means you set all the settings back to None for that groups watches

Even more flexible would be multiple templates for each group...?

@timlod
Copy link
Contributor Author

timlod commented May 19, 2022

The former yes, the latter I hadn't really considered that way, but I guess it would be good if one can sync/erase them at once.
Both could be achieved with the current PR by editing one of the watches and activating the toggle (I think if we want to add it this way at all, I should add a warning that this is a potentially destructive action wrt. the other watches in the group, potentially along with a list of watches which will be modified).

Another way to solve group-level options in a way that you still have control over individual watches, would be to have another level of global settings, but at group level. By the way, here I notice that global filters aren't very visible from the individual watches' level - I think it should be possible to do this with colouring perhaps? (Blue: global filters, green: group-level filters [if we go for it], black: individual filters).

I think we can agree that having templates is a good feature to have in any case. I'm not sure whether having them at the group level would be a good idea though, because it adds a lot of house-keeping (and, I conjecture, may only become useful once there are very many templates)

@dgtlmoon dgtlmoon force-pushed the master branch 3 times, most recently from 3e9e4ee to 19c96f4 Compare June 14, 2022 10:00
@dgtlmoon
Copy link
Owner

I'm still thinking about this PR but haven't quite coalesced on the right direction

@timlod
Copy link
Contributor Author

timlod commented Jun 20, 2022

Let me know if I can help in any way. My current thought is that bulk edit (e.g. this PR), templating and syncing are all useful, but different enough in functionality that I think it should be separated in perhaps several logical features and implemented across different work. However, they may all benefit from some common code (specifically relating to watch groups/tags).

@dgtlmoon
Copy link
Owner

Maybe there can be a new struct in the JSON datastore, with 'groups'

then when accessing the data on the watches as a python property, it can pull the over-riding group info..

I will experiment

@dgtlmoon
Copy link
Owner

dgtlmoon commented Nov 2, 2022

#1064 will soon get merged

the thinking is that then it should be very simple to just extend the Watch model object's getter function and automatically inject/append the various values to the filter/ignore/etc vars as needed instead of doing it with extra code

@dgtlmoon
Copy link
Owner

#1064 is in, so we can now focus on using the model/Watch.py with the filters/selectors etc as a @property that will take any hierarchy

@dgtlmoon dgtlmoon closed this Nov 30, 2022
@dgtlmoon
Copy link
Owner

resolved in #1610

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

2 participants