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

Replace cursor_pagination #148

Closed
dblock opened this issue Mar 26, 2022 · 12 comments · Fixed by #158
Closed

Replace cursor_pagination #148

dblock opened this issue Mar 26, 2022 · 12 comments · Fixed by #158
Labels

Comments

@dblock
Copy link
Collaborator

dblock commented Mar 26, 2022

https://github.com/Kukunin/cursor_pagination will no longer be maintained. I've maintained a fork, but it doesn't make sense long term, https://github.com/slack-ruby/slack-ruby-bot-server/blob/master/Gemfile#L12.

Replace it with another option, e.g. https://github.com/ddnexus/pagy.

@crazyoptimist
Copy link
Collaborator

Let me take this one.

@crazyoptimist
Copy link
Collaborator

Stuck in #155

@crazyoptimist
Copy link
Collaborator

This is my environment setup:

debian @ workspace in slack-ruby-bot-server on  chore/replace-cursor-with-pagy via 💎 v3.1.3 on ☁️  (us-east-2)
⌛ 11:46 ❯ firefox --version
Mozilla Firefox 108.0.1

debian @ workspace in slack-ruby-bot-server on  chore/replace-cursor-with-pagy via 💎 v3.1.3 on ☁️  (us-east-2)
⌛ 11:46 ❯ geckodriver --version
geckodriver 0.32.0 (4563dd583110 2022-10-13 09:22 +0000)

And the integration tests still fail:

  1) Teams oauth v1 oauth registers a team
     Failure/Error: visit '/?v1&code=code'

     Selenium::WebDriver::Error::UnknownError:
       Process unexpectedly closed with status 1
     # ./spec/integration/teams_spec.rb:26:in `block (5 levels) in <top (required)>'
     # ./spec/integration/teams_spec.rb:25:in `block (4 levels) in <top (required)>'
     # ./spec/support/database_cleaner.rb:11:in `block (3 levels) in <top (required)>'
     # ./spec/support/database_cleaner.rb:10:in `block (2 levels) in <top (required)>'

Meanwhile, using this command, all tests pass:
/usr/bin/xvfb-run --auto-servernum bundle exec rake spec

Do you have any advice, @dblock ?

@dblock
Copy link
Collaborator Author

dblock commented Dec 22, 2022

Is this Linux? I think you may need to start Xvfb manually to get a display. This seems to have a reasonable explanation: http://elementalselenium.com/tips/38-headless

@crazyoptimist
Copy link
Collaborator

crazyoptimist commented Dec 28, 2022

I've been playing around replacing cursor with pagy. pagy doesn't support cursor pagination. Are we okay to drop the cursor pagination support for active record?
@dblock

@dblock
Copy link
Collaborator Author

dblock commented Dec 28, 2022

@crazyoptimist I don't think so unless we absolutely must, because cursor pagination is the only way to keep a stable query. I wouldn't use a page by page sort in a production app with changing data.

@crazyoptimist
Copy link
Collaborator

crazyoptimist commented Dec 28, 2022

@dblock pagy-cursor seems to be only one supporting cursor pagination for AR, but I wouldn't recommend to use it because it's immuture yet.

@dblock
Copy link
Collaborator Author

dblock commented Dec 29, 2022

@dblock pagy-cursor seems to be only one supporting cursor pagination for AR, but I wouldn't recommend to use it because it's immuture yet.

I like it enough, let's help mature it?

@crazyoptimist
Copy link
Collaborator

Already tried it and got this issue, I might did something wrong. Nonetheless, the repo doesn't seem to be actively maintained, no responses in several open issues.

@dblock
Copy link
Collaborator Author

dblock commented Dec 29, 2022

Do you have a branch with this non-working code?

@crazyoptimist
Copy link
Collaborator

@crazyoptimist
Copy link
Collaborator

I'm playing with this starter, where slack-ruby-bot-server is locally plugged in. Just FYI.

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

Successfully merging a pull request may close this issue.

2 participants