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

Create pagination for the index page #198 #201

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ gem 'slim'
gem 'hanami-webpack', github: 'samuelsimoes/hanami-webpack'
gem 'sass'
gem 'relative_time'
gem 'hanami-pagination'

gem 'hanami-serializer', github: 'davydovanton/hanami-serializer'

Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ GEM
rom (~> 3.3, >= 3.3.3)
rom-repository (~> 1.4)
rom-sql (~> 1.3, >= 1.3.5)
hanami-pagination (0.1.0)
hanami-model (~> 1.0)
hanami-router (1.1.0)
hanami-utils (~> 1.1)
http_router (= 0.11.2)
Expand Down Expand Up @@ -442,6 +444,7 @@ DEPENDENCIES
hanami (= 1.1.0)
hanami-fabrication
hanami-model (= 1.1.0)
hanami-pagination
hanami-serializer!
hanami-webpack!
hiredis
Expand Down
30 changes: 30 additions & 0 deletions apps/web/assets/stylesheets/_tasks.scss
Original file line number Diff line number Diff line change
Expand Up @@ -543,3 +543,33 @@
margin: 0 -10px -7px;
}
}

.pagination {
text-align: center;
padding: 0.3em;
cursor: default; }
.pagination a, .pagination span, .pagination em {
padding: 0.2em 0.5em; }
.pagination .disabled {
color: #aaaaaa; }
.pagination .current {
font-style: normal;
font-weight: bold;
color: red; }
.pagination a {
border: 1px solid #dddddd;
color: #214CFD;
text-decoration: none; }
.pagination a:hover, .pagination a:focus {
border-color: #003366;
background: #214CFD;
color: white; }
.pagination .page_info {
color: #aaaaaa;
padding-top: 0.8em; }
.pagination .previous_page, .pagination .next_page {
border-width: 2px; }
.pagination .previous_page {
margin-right: 1em; }
.pagination .next_page {
margin-left: 1em; }
8 changes: 7 additions & 1 deletion apps/web/controllers/tasks/index.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
module Web::Controllers::Tasks
class Index
include Web::Action
include Hanami::Pagination::Action
expose :tasks

def call(params)
@tasks = TaskRepository.new.find_by(search_params)
repo = TaskRepository.new.find_by(search_params)
@tasks = all_for_page(repo)
Copy link
Member

Choose a reason for hiding this comment

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

it's not a repo 🤔
let's rename it to relation

end

private
Expand All @@ -29,5 +31,9 @@ def with_language(search_params)
def status
Task::VALID_STATUSES.values.include?(params[:status]) ? params[:status] : 'in progress'
end

def limit
10
Copy link
Member

Choose a reason for hiding this comment

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

let's make limit as 50?

end
end
end
4 changes: 4 additions & 0 deletions apps/web/templates/tasks/index.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
.tasks__language-selector
= select_tasks_by_language

#pagination
- if pager.all_pages.count > 1
= pagination

.tasks__new
- if current_user.registred?
= link_to_new_task
Expand Down
81 changes: 81 additions & 0 deletions apps/web/views/tasks/index.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module Web::Views::Tasks
class Index
include Web::View
include Hanami::Pagination::View

def title
'OSSBoard: tasks'
Expand Down Expand Up @@ -76,5 +77,85 @@ def author_information(author, task)
text(" • #{task.lang}")
end
end

def pagination
html.div(class: 'pagination') do
Copy link
Member

Choose a reason for hiding this comment

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

we really need all this code without any specs here?

Copy link
Author

Choose a reason for hiding this comment

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

I wrote this code, because pagination function was not available in hanami-pagination gem.
Now i replaced it with gem function.


if pager.prev_page.nil?
span(class: 'disabled previous_page')
text '← Previous'
else
a(href: page_url(pager.prev_page) + '&status=' + params[:status]) do
text '← Previous'
end
end

total_pages = pager.all_pages.count

first_in_range = pager.pages_range.first

if 1 < first_in_range
a(href: page_url(1) + '&status=' + params[:status]) do
1
end
end

if 2 < first_in_range
a(href: page_url(2) + '&status=' + params[:status]) do
2
end
end

if 1 < first_in_range || 2 < first_in_range
span(class: 'gap') do
text '...'
end
end

pager.pages_range.each do |page_number|

if pager.current_page?(page_number)
em(class: 'current') do
text page_number
end
else
a(href: page_url(page_number) + '&status=' + params[:status]) do
page_number
end
end

end

last_in_range = pager.pages_range[-1]

if last_in_range < total_pages
span(class: 'gap') do
text '...'
end

if last_in_range != total_pages-1
a(href: page_url(total_pages-1) + '&status=' + params[:status]) do
total_pages-1
end
end

a(href: page_url(total_pages) + '&status=' + params[:status]) do
total_pages
end

end

if pager.next_page.nil?
span(class: 'disabled next_page')
text 'Next →'
else
a(href: page_url(pager.next_page) + '&status=' + params[:status]) do
text 'Next →'
end
end

end
end

end
end
1 change: 1 addition & 0 deletions config/initializers/enable_pagination.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
TaskRepository.enable_pagination!
2 changes: 1 addition & 1 deletion lib/ossboard/repositories/task_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def all_from_date_counted_by_status_and_day(from)
end

def find_by(params = {})
tasks.where(params).order { id.desc }.map_to(Task).to_a
tasks.where(params).order { id.desc }.map_to(Task)
Copy link
Member

Choose a reason for hiding this comment

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

WDYT if we split this code into two methods:

def find_by(params = {})
  search_relation(params).to_a
end

def search_relation(params)
  tasks.where(params).order { id.desc }.map_to(Task)
end

end

def assigned_tasks_for_user(user)
Expand Down
16 changes: 8 additions & 8 deletions spec/ossboard/repositories/task_repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@

context 'when params is empty' do
it 'returns array of closed tasks' do
result = repo.find_by(approved: true)
result = repo.find_by(approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 3
end
end

context 'when status is closed' do
it 'returns array of closed tasks' do
result = repo.find_by(status: 'closed', approved: true)
result = repo.find_by(status: 'closed', approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 1
expect(result.first.status).to eq 'closed'
Expand All @@ -27,7 +27,7 @@

context 'when status is done' do
it 'returns array of done tasks' do
result = repo.find_by(status: 'done', approved: true)
result = repo.find_by(status: 'done', approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 1
expect(result.first.status).to eq 'done'
Expand All @@ -36,7 +36,7 @@

context 'when status is in progress' do
it 'returns array of in progress tasks' do
result = repo.find_by(status: 'in progress', approved: true)
result = repo.find_by(status: 'in progress', approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 1
expect(result.first.status).to eq 'in progress'
Expand All @@ -45,7 +45,7 @@

context 'when lang is ruby' do
it 'returns array of ruby language tasks' do
result = repo.find_by(lang: 'ruby', approved: true)
result = repo.find_by(lang: 'ruby', approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 1
expect(result.first.lang).to eq 'ruby'
Expand All @@ -54,7 +54,7 @@

context 'when lang is haskell' do
it 'returns array of haskell language tasks' do
result = repo.find_by(lang: 'haskell', approved: true)
result = repo.find_by(lang: 'haskell', approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 1
expect(result.first.lang).to eq 'haskell'
Expand All @@ -63,7 +63,7 @@

context 'when lang is unknown' do
it 'returns array of unknown language tasks' do
result = repo.find_by(lang: 'unknown', approved: true)
result = repo.find_by(lang: 'unknown', approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 1
expect(result.first.lang).to eq 'unknown'
Expand All @@ -72,7 +72,7 @@

context 'when status is in progress and lang is ruby' do
it 'returns array of in progress, ruby tasks' do
result = repo.find_by(status: 'in progress', lang: 'ruby', approved: true)
result = repo.find_by(status: 'in progress', lang: 'ruby', approved: true).to_a
expect(result).to all(be_a(Task))
expect(result.count).to eq 1
expect(result.first.lang).to eq 'ruby'
Expand Down
35 changes: 35 additions & 0 deletions spec/web/controllers/tasks/index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,41 @@
expect(action.tasks.map(&:status)).to all(eq('in progress'))
end
end

context 'when are on the first page' do
Copy link
Member

Choose a reason for hiding this comment

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

let's simplify specs here. For this we can use DI (dependency injection and wiki) for repository instance:

So, I found a problem in pagination davydovanton/hanami-pagination#10, thanks for this! 💯 🌟 🎉

let(:params) { { lang: 'ruby', status: 'done', page: 1} }

before do
8.times { |i| Fabricate.create(:task, title: "title ##{i}", approved: true, status: 'done', lang: 'ruby') }
action.call(params)
end

it 'returns 10 tasks on page' do

Copy link
Member

Choose a reason for hiding this comment

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

missing space here

expect(action.tasks).to all(be_a(Task))
expect(action.tasks.count).to eq 10
expect(action.tasks.map(&:status)).to all(eq('done'))
end

end

context 'when are on the second page' do
let(:params) { { lang: 'ruby', status: 'done', page: 2} }

before do
8.times { |i| Fabricate.create(:task, title: "title ##{i}", approved: true, status: 'done', lang: 'ruby') }
action.call(params)
end

it 'returns 1 task on page' do
expect(action.tasks).to all(be_a(Task))
expect(action.tasks.count).to eq 1
expect(action.tasks.map(&:status)).to all(eq('done'))
end

end


Copy link
Member

Choose a reason for hiding this comment

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

could you cut unnecessary spaces here and format specs? thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Ok, i will focus on it more.

end
end
end