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 server-side processing support for SearchBuilder #1113

Merged
merged 8 commits into from
Jan 20, 2024

Conversation

mikmart
Copy link
Contributor

@mikmart mikmart commented Jan 20, 2024

Fixes #963.

This PR adds support for SearchBuilder in server-side processing. I did not add support for the moment or luxon column types as I'm not sure how they map to R types. The array column type is not supported either.

Notably datetime columns are treated as character data. This seems to be a limitation with the SearchBuilder extension.

I was not sure how to map the null or empty conditions into R. The approach I chose was to map them to missing values, and, for character columns, also to empty strings.

I added some unit tests to confirm that the main components in the processing work as intended, but did not attempt to get thorough coverage on all cases. In addition to the unit tests, I used this app for some further manual testing:

library(shiny)

shinyApp(
  fluidPage(DT::DTOutput('foo')),
  function(input, output) {
    output$foo = DT::renderDT(
      data.frame(
        a = sample(26), b = letters,
        c = factor(rep(c('a', 'b'), 13)),
        d = Sys.Date() + 1:26,
        e = Sys.time() + 1000 * (1:26)
      ),
      options = list(dom = 'Qlfrtip'),
      extensions = c('SearchBuilder', 'DateTime')
    )
  }
)

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

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

Wow! This is incredible. Thank you so much!

Please add a news item and I think it'll be ready to merge.

@mikmart
Copy link
Contributor Author

mikmart commented Jan 20, 2024

Happy to help. And thank you for blogging about this! You were right, this was a fun project.

I added the NEWS item and also:

  1. Verified that both empty and NA strings count for null in client-side processing.
  2. Encoded the string null evaluation rules in a unit test.
  3. Removed the second logic evaluation test that was, on second thought, a bit redundant.

@mikmart mikmart mentioned this pull request Jan 20, 2024
@yihui yihui merged commit bd984f8 into rstudio:main Jan 20, 2024
10 checks passed
AhmedKhaled945 pushed a commit to AhmedKhaled945/DT that referenced this pull request Feb 2, 2024
@AhmedKhaled945
Copy link

Thanks a lot for the work @mikmart , @yihui

Just a question that i cannot find an answer to, i used the code above to test, works like a charm, but for the factor column (c), when i try to add a condition, it is giving me a textbox instead of dropdown of choices, is this how it is supposed to be? or should i add another option somewhere to let me choose from fixed values (since it is a factor),

image

Thanks in advance,

@mikmart
Copy link
Contributor Author

mikmart commented Feb 2, 2024

Thanks for trying it out @AhmedKhaled945! What you're seeing is actually a documented feature of the SearchBuilder extension in server-side processing mode (emphasis added):

There are two caveats that SearchBuilder's server-side integration carries. The first is that anywhere select elements would normally be used on the client-side, input elements are used instead. This reduces strain on the server significantly, drastically improving performance.

I'm not aware of any straightforward way to change this behaviour.

@AhmedKhaled945
Copy link

Ok got it, thanks a lot @mikmart

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FR] SearchBuilder works with Server-Side processing mode
3 participants