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

Spam2: pagination added #8063

Merged
merged 13 commits into from
Jun 30, 2020
Merged

Spam2: pagination added #8063

merged 13 commits into from
Jun 30, 2020

Conversation

keshavsethi
Copy link
Member

Stable was taking too much time to render all data. So, here I used Will_Paginate which will divide data into pages. I will customize it more in future PRs. Please refer to the following Screenshot.

Screenshot from 2020-06-24 03-54-53

@jywarren @emilyashley @cesswairimu @pydevsg @VladimirMikulic Please review.
Thanks!

@@ -11,7 +11,12 @@ function table_main(id) {
"search": {
"regex": true
},
"scrollX": true
"scrollX": true,
"paging":false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please format JS properly (with a tool)?
Thanks, @keshavsethi.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ya Sure, Thanks!

@codecov
Copy link

codecov bot commented Jun 23, 2020

Codecov Report

Merging #8063 into master will increase coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8063      +/-   ##
==========================================
+ Coverage   82.41%   82.53%   +0.12%     
==========================================
  Files          99       99              
  Lines        5737     5737              
==========================================
+ Hits         4728     4735       +7     
+ Misses       1009     1002       -7     
Impacted Files Coverage Δ
app/controllers/spam2_controller.rb 100.00% <100.00%> (ø)
app/controllers/tag_controller.rb 81.40% <100.00%> (ø)
app/api/srch/search.rb 70.06% <0.00%> (+3.82%) ⬆️
app/services/execute_search.rb 94.44% <0.00%> (+5.55%) ⬆️

app/controllers/spam2_controller.rb Outdated Show resolved Hide resolved
app/controllers/spam2_controller.rb Outdated Show resolved Hide resolved
@keshavsethi keshavsethi requested a review from jywarren June 24, 2020 15:10
@keshavsethi
Copy link
Member Author

@jywarren I will customize pagination more in future PRs. Thanks!!

@jywarren
Copy link
Member

Great, thank you!!!

@keshavsethi
Copy link
Member Author

keshavsethi commented Jun 24, 2020

@jywarren I have divided nodes into a batch of 100 which means all nodes will be rendered in batches. we can use table filters and search and other bulk features within that batch. It will reduce load time without hindering any feature. I have also ordered nodes wrt time. For example, there are 1000 nodes then it will be divided into a batch of 100 so there are 10 batches in total. Within each batch, all filters and stats will work. There are few Screenshots Please review
Screenshot from 2020-06-24 21-55-28
Screenshot from 2020-06-24 21-56-40
Thanks!!

@jywarren
Copy link
Member

Hm, looks like tests didn't pass? Can you check what happened? Thanks!

@keshavsethi
Copy link
Member Author

keshavsethi commented Jun 24, 2020

Hm, looks like tests didn't pass? Can you check what happened? Thanks!

All four tests in Travis are failing

bundler: command not found: rails
Install missing gem executables with `bundle install`
The command "bundle exec rails test test/integration" exited with 127.

@jywarren, please help. I don't know why it is happening. Do I need to install some gems?? Tests are working fine locally. Thanks!!

@cesswairimu
Copy link
Collaborator

Yeah that is weird..trying to restart travis

@cesswairimu cesswairimu reopened this Jun 24, 2020
@cesswairimu
Copy link
Collaborator

cesswairimu commented Jun 24, 2020

Hi @keshavsethi seems the build failed on master after the last merge...checking to see if I can get that fixed

@keshavsethi
Copy link
Member Author

Hi @keshavsethi seems the build failed on master after the last merge...checking to see if I get that fixed

@cesswairimu @jywarren Thanks!! Should I close and reopen this PR??

@cesswairimu
Copy link
Collaborator

Sure, you could try that...I opened a PR #8074 that I am trying to fix this error, no luck yet though

@cesswairimu
Copy link
Collaborator

@keshavsethi you can rebase or reopen this...the errors are fixed

@keshavsethi keshavsethi reopened this Jun 27, 2020
@keshavsethi
Copy link
Member Author

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.

🎉

@cesswairimu
Copy link
Collaborator

Hi @VladimirMikulic, could you please give a final review when you get a minute against the changes you requested? Thanks

app/views/spam2/_comments.html.erb Show resolved Hide resolved
app/views/spam2/_comments.html.erb Outdated Show resolved Hide resolved
app/assets/stylesheets/spam2.css Outdated Show resolved Hide resolved
keshavsethi and others added 2 commits June 29, 2020 21:12
@pydevsg pydevsg self-requested a review June 29, 2020 19:15
@pydevsg
Copy link
Member

pydevsg commented Jun 29, 2020

Great work @keshavsethi 🎉

@cesswairimu cesswairimu merged commit 7a5c0bd into publiclab:master Jun 30, 2020
@cesswairimu
Copy link
Collaborator

This is awesome 🚀 thanks a lot everyone 🎉 🎉 🎉

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.

5 participants