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

Fixed Pagination issue #9330

Merged
merged 18 commits into from
Mar 23, 2021
Merged

Fixed Pagination issue #9330

merged 18 commits into from
Mar 23, 2021

Conversation

gaurav2699
Copy link
Contributor

@gaurav2699 gaurav2699 commented Mar 19, 2021

Fixes #9290 (<=== Add issue number here)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!
image
image

@gitpod-io
Copy link

gitpod-io bot commented Mar 19, 2021

@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@1b54747). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head e5f16aa differs from pull request most recent head 64bdb76. Consider uploading reports for the commit 64bdb76 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9330   +/-   ##
=======================================
  Coverage        ?   82.09%           
=======================================
  Files           ?       98           
  Lines           ?     5870           
  Branches        ?        0           
=======================================
  Hits            ?     4819           
  Misses          ?     1051           
  Partials        ?        0           

@gaurav2699
Copy link
Contributor Author

@cesswairimu @jywarren please review

@RuthNjeri
Copy link
Contributor

Hi, @gaurav2699 also noting this issue #714 that aims to remove all references to tool and place, which could render your code obsolete

@gaurav2699
Copy link
Contributor Author

@RuthNjeri You can check now. Thanks!

@gaurav2699 gaurav2699 closed this Mar 19, 2021
@gaurav2699 gaurav2699 reopened this Mar 19, 2021
@gitpod-io
Copy link

gitpod-io bot commented Mar 19, 2021

@gaurav2699 gaurav2699 closed this Mar 20, 2021
@gaurav2699 gaurav2699 reopened this Mar 20, 2021
@gitpod-io
Copy link

gitpod-io bot commented Mar 20, 2021

Copy link
Collaborator

@Tlazypanda Tlazypanda left a comment

Choose a reason for hiding this comment

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

@gaurav2699 LGTM 🎉 can we also write a functional test to make sure this doesn't break later?

@gaurav2699
Copy link
Contributor Author

@Tlazypanda I added a functional test, please review and check if it's alright! Thanks a lot!

@cesswairimu
Copy link
Collaborator

Hi @gaurav2699 , there are one test that was added on main that is failing because of the new wikis created in your test block...would you like to help make these a little more dynamic this would involve changing this test to smth like

test 'should sort by last edited' do
   get :index, params: { sort: "last_edited" }
   wikis = assigns(:wikis)
   wiki1 = wikis.order(changed: :desc).first
   wiki2 = wikis.order(changed: :desc).reverse.first
   assert_equal(wikis.first.title, wiki1.title)
   assert_equal(wikis.last.title, wiki2.title)
end

what do you think? Thanks

@codeclimate
Copy link

codeclimate bot commented Mar 23, 2021

Code Climate has analyzed commit 64bdb76 and detected 0 issues on this pull request.

View more on Code Climate.

@gaurav2699
Copy link
Contributor Author

Thanks for the guidance @cesswairimu. I made the change and now the functional tests doesn't fail. Please review. Thanks a lot.

@cesswairimu
Copy link
Collaborator

Fantastic thanks 🎉

Copy link
Collaborator

@cesswairimu cesswairimu left a comment

Choose a reason for hiding this comment

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

Great great job here @gaurav2699 thanks a lot

@cesswairimu cesswairimu merged commit d70efef into publiclab:main Mar 23, 2021
reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this pull request Oct 16, 2021
* Added translations in users/settings.html.erb

* Fixed pagination issue

* fixed pagination issue

* Fixed pagination in wiki/stale too

* Delete identifier.sqlite

* fixed a minor issue

* Fixed minor issue

* Delete identifier.sqlite

* Added functional test to check pagination

* reduced no of new wikis for test

* reduced 31 to 12

Co-authored-by: Cess <cessmbuguar@gmail.com>
billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this pull request Dec 28, 2021
* Added translations in users/settings.html.erb

* Fixed pagination issue

* fixed pagination issue

* Fixed pagination in wiki/stale too

* Delete identifier.sqlite

* fixed a minor issue

* Fixed minor issue

* Delete identifier.sqlite

* Added functional test to check pagination

* reduced no of new wikis for test

* reduced 31 to 12

Co-authored-by: Cess <cessmbuguar@gmail.com>
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.

Sorting based on edits / page views / likes shows only 1 row on the first page of wikis
4 participants