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

Сторінка з рейтингом найкращих звітів #134

Open
wants to merge 28 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@lynxrv21
Copy link
Contributor

lynxrv21 commented Feb 14, 2018

Повний опис тут #133

lynxrv21 added some commits Feb 9, 2018

@lynxrv21 lynxrv21 requested a review from ttp Feb 14, 2018

@stal1n274

This comment has been minimized.

Copy link
Contributor

stal1n274 commented Feb 14, 2018

👍

@ttp ttp changed the title Reports competition Сторінка з рейтингом найкращих звітів Feb 16, 2018

@stal1n274

This comment has been minimized.

Copy link
Contributor

stal1n274 commented Feb 24, 2018

👍

def show
@competition = Competition.find(params[:id])
@post = @competition.post
@results = CompetitionResult.where(competition_id: params[:id])

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

ці додаткові змінні не потрібні, просто використовуй @competition.post & @competition.competition_results

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Ок


enum status: [:open, :closed, :rejected]

def close()

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

def close без дужок

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Done

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

done коли запушано, до того моменту не done

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Це ще в попередньому комміті було done :)

This comment has been minimized.

Copy link
@ttp

ttp Mar 2, 2018

Contributor

done коли запушано, доки я бачу тут цей тред, воно не запушано

top_reports = Report.where.not(created_by: nil).where.not(
rating_score: 0).where.not(historical: true).where(
created_at: (start_at..end_at)).order(
rating_score: :desc).limit(5)

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

хаос форматування

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

https://github.com/bbatsov/ruby-style-guide#consistent-multi-line-chains
use Option A

Report.where.not(created_by: nil)
  .where.not(rating_score: 0)
  .where.not(historical: true)
  .where(created_at: start_at..end_at)
  .order(rating_score: :desc)
  .limit(5)

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Порядок повернуто


top_reports.each_with_index do |report, index|
CompetitionResult.create(competition: self, report: report,
score: report.rating_score, position: index + 1)

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

форматування

This comment has been minimized.

Copy link
@stal1n274

stal1n274 Feb 28, 2018

Contributor

а що саме тут з форматуванням не так?

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

воно відсутнє

  1. або все візьми в один рядок
    CompetitionResult.create(competition: self, report: report, score: report.rating_score, position: index + 1)
  2. або все з нового рядка
CompetitionResult.create(
  competition: self,
  report: report,
  score: report.rating_score,
  position: index + 1
)
  1. або вирівняй по першому ключу
CompetitionResult.create(competition: self, report: report,
                         score: report.rating_score, position: index + 1)
# or
CompetitionResult.create(competition: self,
                         report: report,
                         score: report.rating_score,
                         position: index + 1)

я надаю перевагу 1. коли рядок не довгий, або 2. коли багато ключів

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Порядок повернуто [1]

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Дякую за пояснення 👍

наприклад: Став був клубним і ловити було заборонено,
лише обраним по запрошеннях. Проте тепер ставок відкритий для
ловлі для всіх бажаючих. Отже, ставок знаходиться десь
за 150-200 м справа від в'їзду в с.Півче зі сторони

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

відміни ці зміни

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Trailing spaces are back!

@@ -32,6 +32,7 @@
post :unvote
end
end
resources :competitions

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

так ти оголосиш надлишкові роути, використай only: [...], див приклади вище

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Змінив на resources :competitions, only: [:index, :show]

t.datetime "results_at", null: false
t.string "postable_type"
t.integer "postable_id"
t.integer "status", null: false

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

дай значення по замовчуванню для статуса

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Додав

t.datetime :end_at, null: false
t.datetime :results_at, null: false
t.references :postable, polymorphic: true, index: true
t.integer :status, null: false

This comment has been minimized.

Copy link
@ttp

ttp Feb 28, 2018

Contributor

статус має мати значення по замовчуванню

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Feb 28, 2018

Author Contributor

Додав

lynxrv21 added some commits Feb 28, 2018

@stal1n274

This comment has been minimized.

Copy link
Contributor

stal1n274 commented Mar 1, 2018

єє!

.pull-right
= link_to t('helpers.links.read_more'), competition_path(competition.id), class: 'btn btn-default'

= will_paginate @posts

This comment has been minimized.

Copy link
@ttp

ttp Mar 2, 2018

Contributor

@competitions

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 2, 2018

Author Contributor

Fixed

h3.page-header.clearfix
= link_to competition.post.title, competition_path(competition.id)
.pull-right
small= t("competitions.#{competition.status}")

This comment has been minimized.

Copy link
@ttp

ttp Mar 2, 2018

Contributor

t(competition.status, scope: 'competitions')

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 2, 2018

Author Contributor

Updated

@@ -26,6 +26,14 @@
i.glyphicon.glyphicon-edit
h1.page-header= @report.title

div class="byline"
- if @report.author
address class="author"

This comment has been minimized.

Copy link
@ttp

ttp Mar 7, 2018

Contributor

якось нелогічно юзати тег адреси для автора

This comment has been minimized.

Copy link
@ttp

ttp Mar 7, 2018

Contributor

також юзай скорочення заміть атрибуту class

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 7, 2018

Author Contributor

Знайшов на SO
Можна юзати <span class="author">John</span> чи <address class="author">By <a rel="author" href="/author/john-doe">John Doe</a></address>

This comment has been minimized.

Copy link
@ttp

ttp Mar 7, 2018

Contributor

юзай спан, це ж не адреса

This comment has been minimized.

Copy link
@ttp

ttp Mar 7, 2018

Contributor

короче так, спека пише що можна і для контактів юзати https://www.w3.org/TR/html50/sections.html#the-address-element

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 7, 2018

Author Contributor

і + коли буде сторінка юзера/всі звіти автора то це буде лінка і тоді тим більше матиме сенс: адреса профіля автора

.row
.col-md-9
- @competitions.each do |competition|
.fish-list-item.clearfix

This comment has been minimized.

Copy link
@ttp

ttp Mar 7, 2018

Contributor

.fish-list-item

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 7, 2018

Author Contributor

Пофіксив

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 7, 2018

Author Contributor

До речі, чому тут без clearfix?
В звітах і постах цей клас є, без нього між конкурсами немає відступу.

- if @report.author
address class="author"
= t('reports.author') + ': '
= @report.author.full_name + ' | '

This comment has been minimized.

Copy link
@ttp

ttp Mar 7, 2018

Contributor

я думаю по феньшую треба тоді розбити сам тег автора від лейби

address
  = t('reports.author') + ': '
  span.author= @report.author.full_name
= ' | '

щоб автор містив лише ім"я

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 7, 2018

Author Contributor

Маєш рацію. Поправлю

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 7, 2018

Author Contributor

Проапдейтив

@lynxrv21

This comment has been minimized.

Copy link
Contributor Author

lynxrv21 commented Mar 7, 2018

@ttp чому чеки падають, там якісь тести зав'язані на верстку?

@@ -0,0 +1,50 @@
= page_title @page_title || @competition.post.title
- if policy(@competition.post).edit?

This comment has been minimized.

Copy link
@ttp

ttp Mar 12, 2018

Contributor

Всі чеки і шляхи мають прив"язуватись до competition а не до post, виправ тут і нижче

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 12, 2018

Author Contributor

Тут виправив. Більше не знайшов де.

span.author= result.report.author.full_name
= ' | '
time
= l(result.report.created_at.to_date)

This comment has been minimized.

Copy link
@ttp

ttp Mar 12, 2018

Contributor

не перенось текст від тегу якщо це просто текст, пиши в один рядок, виправ тут і скрізь по розмітці

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 12, 2018

Author Contributor

ОК

- if result.report.available_featured_image
img src=result.report.available_featured_image[:post_thumb].url class=(result.position == 1? 'post-thumbnail full-width' : 'post-thumbnail')
.well
span class=(result.position == 1? 'h3' : 'h4')

This comment has been minimized.

Copy link
@ttp

ttp Mar 12, 2018

Contributor

використовуй блочний елемент для блочних стилів, div

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 12, 2018

Author Contributor

Yes

@@ -198,7 +204,7 @@ uk:
лише обраним по запрошеннях. Проте тепер ставок відкритий для
ловлі для всіх бажаючих. Отже, ставок знаходиться десь
за 150-200 м справа від в'їзду в с.Півче зі сторони
Мізоча (8 км від Мізоча). Ставок чистий, незарозший, з твердим дном
Мізоча (8 км від Мізоча). Ставок чистий, незарозший, з твердим дном

This comment has been minimized.

Copy link
@ttp

ttp Mar 12, 2018

Contributor

revert

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 12, 2018

Author Contributor

Done

@@ -32,6 +32,8 @@
post :unvote
end
end
resources :competitions, only: [:index, :show] do
end

This comment has been minimized.

Copy link
@ttp

ttp Mar 12, 2018

Contributor

блок не треба передавати якщо він порожній

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 12, 2018

Author Contributor

Ти про do - end? Ок, але таке є ще тут

This comment has been minimized.

Copy link
@ttp

ttp Mar 12, 2018

Contributor

так склалося історично )


.col-md-3.sidebar
p
a.btn.btn-primary.btn-lg.add( href='/posts/4090/' )= t('competitions.competition_rules')

This comment has been minimized.

Copy link
@ttp

ttp Mar 15, 2018

Contributor

ніяких захардкоджених id! Що буде на інших енвайрментах де нема цієї публікації?

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 15, 2018

Author Contributor

На проді є. Що туди прописати?

This comment has been minimized.

Copy link
@ttp

ttp Mar 15, 2018

Contributor

НІКОЛИ не слухай і не роби те що каже Вітя в плані коду і як то робити
Яка різниця, що воно є на проді, коли ні локально ні на тестовому енвайрменті воно не буде працювати? Всі такі значення мають виноситись або в конфіг або в бд де вони редагуються через адмінку.

This comment has been minimized.

Copy link
@stal1n274

stal1n274 Mar 15, 2018

Contributor

я не стверджував хардкодити айдішніки
з мене треба було лише написати публікацію з правилами

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 15, 2018

Author Contributor

Срач в коментах - безцінно!

This comment has been minimized.

Copy link
@ttp

ttp Mar 15, 2018

Contributor

тут є наступні варіанти:

  1. Зробити конфіг значення для цього, мені не подобається, бо доведеться всеодно редагувати то локально щоб заставити працювати.
  2. Шукати post не по id який неможливо відредагувати, а по slug. Додати автоматичне створення цього поста в db/seeds.rb. Додати індекс для slug поля для постів.
  3. Додати цей id в settings, автоматично створювати пост і ключ в базі для цього в db/seeds.rb

На даний момент 3 виглядає найпростішим.

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 15, 2018

Author Contributor

Тоді всеодно буде захардкоджений id. Спробую 2, хоча автоматичне створення мені не подобається. Пихати туди всю простиню правил?

This comment has been minimized.

Copy link
@ttp

ttp Mar 15, 2018

Contributor

Він не буде захардкоджений в будь якому з цих варіантів. Ти маєш шукати пост по значенню з сеттігів і виводити кнопку лише у тому випадку коли такий пост існує і опублікований

This comment has been minimized.

Copy link
@ttp

ttp Mar 15, 2018

Contributor

Ти лише створюєш пост, а не забиваєш контент для нього, решту робить Вітя з адмінки. Пиши всі питання в телеграм, не люблю простині коментів на гітхабі

class Competition < ApplicationRecord
has_one :post, as: :postable
has_many :competition_results

This comment has been minimized.

Copy link
@ttp

ttp Mar 15, 2018

Contributor

додай валідацію на присутність дат

This comment has been minimized.

Copy link
@ttp

ttp Mar 15, 2018

Contributor

особливо на поля помічені як not null

This comment has been minimized.

Copy link
@lynxrv21

lynxrv21 Mar 15, 2018

Author Contributor

Скинь приклад, будь ласка.

This comment has been minimized.

@chyvonomys

This comment has been minimized.

Copy link

chyvonomys commented Mar 15, 2018

ого тут жизнь

@@ -2,6 +2,8 @@ class CompetitionsController < ApplicationController

def index
@competitions = Competition.order(:end_at).paginate(page: params[:page])
rules_post_id = Settings.value('competition_rules_post')
@rules_post = Post.find(rules_post_id)

This comment has been minimized.

Copy link
@ttp

ttp Mar 28, 2018

Contributor

.find_by(id: rules_post_id) інакше ти отримаєш ексепшн якщо такого поста нема

db/seeds.rb Outdated
rules_post_setting = Settings.find_or_create_by!(key: 'competition_rules_post')
if rules_post_setting.value.nil?
rules_post = Post.create(title: 'Конкурс звітів', content: 'Правила конкурсу звітів.')
rules_post_setting.value = rules_post.id

This comment has been minimized.

Copy link
@ttp

ttp Mar 28, 2018

Contributor

2 space indentation

This comment has been minimized.

Copy link
@ttp

ttp Mar 28, 2018

Contributor

воно не збережеться автоматично, ти маєш викликати .save або використай .update_attribute(...) або .update(value: rules_post.id)

@@ -31,5 +31,6 @@


.col-md-3.sidebar
p
a.btn.btn-primary.btn-lg.add( href='/posts/4090/' )= t('competitions.competition_rules')
- if @rules_post

This comment has been minimized.

Copy link
@ttp

ttp Mar 28, 2018

Contributor

&& @rules_post.published?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.