Skip to content

SearchPanes and SearchBuilder do not consider pre-filtering of the table. #11

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
gotmoo opened this issue Jan 12, 2024 · 8 comments · May be fixed by #12
Open

SearchPanes and SearchBuilder do not consider pre-filtering of the table. #11

gotmoo opened this issue Jan 12, 2024 · 8 comments · May be fixed by #12

Comments

@gotmoo
Copy link
Contributor

gotmoo commented Jan 12, 2024

When you apply a top level filter on a table, this is not taken into account by the options presented by SearchBuilderOptions or SearchPaneOptions.

Setup

Using the current example download for Editor. Showing this with SearchPanes because it is easier to see in the browser:

  1. Modify Controllers\SearchPanesController.cs to add a top level filter on site using the Where clause
var response = new Editor(db, "users")
    .Model<UploadManyModel>()
    .Where("site", "1", "=")
  1. Run the project and visit /examples/extensions/searchPanes.html

Result

The resulting table shows the correct data set:
image

However, the SearchPanes options still show the options from the full data set:
image

This includes the other options like Name:
image

Selecting one of the pre-filtered names results in zero records found, while the options show there should be one.

Expected behavior

Pre-filtered items are hidden from the SearchPane/SearchBuilder options.

This could result in data leaking, or weird results from logically deleted records.

@gotmoo
Copy link
Contributor Author

gotmoo commented Jan 12, 2024

I found a fairly simple solution, that appears to work pretty good:

1st, in editor.cs, expose the _GetWhere method internally:

/// <summary>
/// Apply the global Where filter to the supplied Query
/// </summary>
internal void GetGlobalWhere(Query query)
{
    _GetWhere(query);
}

Then in SearchPaneOptions, apply the filtering twice by calling the above function with the query:

var q = db.Query("select")
    .Distinct(true)
    .Table(table)
    .Get(label + " as label")
    .Get(value + " as value")
    .GroupBy(value)
    .Where(_where)
    .LeftJoin(join);
editor.GetGlobalWhere(q);

and again later:

var entriesQuery = db.Query("select")
    .Distinct(true)
    .Table(table)
    .LeftJoin(join);
editor.GetGlobalWhere(entriesQuery);

Similarly, in SearchBuilder:

var query = db.Query("select")
    .Table(this._table)
    .LeftJoin(_leftJoin);
editor.GetGlobalWhere(query);

If this looks reasonable, I've pushed this onto my fork and created a pull request for this.

See changes here:
Editor.cs
SearchPaneOptions.cs and SearchBuilderOptions.cs

@gotmoo
Copy link
Contributor Author

gotmoo commented Jan 25, 2024

Gentle nudge for @AllanJard to take a look. I hope that may be included in the January update for the NuGet package.

@AllanJard
Copy link
Contributor

Apologies - I've got it in my inbox, but haven't had a chance to have a detailed look yet. I thought I'd added a comment here already, but apparently no - apologies! I can see the benefit of this approach - I'm not sure it is quite the right way to do it, as I think the API is a little confusing. I need to have a bit of a think about it, and it would need to be ported to our other server-side libraries as well. I fear it is unlikely to get in for the Editor 2.3 release, sorry.

@gotmoo
Copy link
Contributor Author

gotmoo commented Jan 25, 2024

No worries, this was a first draft approach that seems to work for me, but there's other ways to solve this. On top of that, my approach does not account for linked tables so that results from a linked table are not filtered against that subset. For example filtering on the ID of a site on the main table would not filter only to that site ID from the sites table.

If you have any suggestions, I could try to work out an alternative way to accomplish the same, perhaps by doing this directly in the Query classes, and basing it on established joins and table names.

I'll also have a think on the API naming for this.

@AllanJard
Copy link
Contributor

I'm wondering about a method on the Options class .UseHostWhere() or something (that's a horrible name, I'm sure I can think of better than that...!). That would then give the flexibility using or not the main Where statement. Or it might be that we should just carry on requiring that it be explicitly set (it could be put into a function that is shared between all Options instances for example).

@gotmoo
Copy link
Contributor Author

gotmoo commented Mar 11, 2024

I will try to find some time to add this to the options class. How about .UseParentWhere() or .ApplyParentWhere()?

What are your thoughts about setting this option in the inverse? Adding an option to explicitly ignore the top level where statement and include all the data? I personally always forget about the options class, and with this the default would be that the search options do not include the pre-filtered data. Then if you do need it included, you can enable that with the options.

@AllanJard
Copy link
Contributor

Given that the library is already deployed and used in production, for backwards compatibility the new feature should be opt in, rather than opt out. So .ParentWhere(true) or something like that would probably be the correct thing to do.

Sorry I've not had a chance to look into this yet. Too many things to do :-)

@gotmoo
Copy link
Contributor Author

gotmoo commented Mar 12, 2024

Agreed on not breaking existing deployments.

No worries on the timing, I know things are crazy with V2 and all that entails! I hope to find some time myself soon to have poke through the code for implementing this as well.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants