Filters by remote_availibity, location, position_type in query object #39

asusikov · 2019-06-24T19:42:45Z

What this PR do?

add to vacancy query object few filters: remote_availibity, location, position_type (just ability to filter, not ui controls)

Related to #33 and #38

apps/web/controllers/vacancies/index.rb

lib/core/queries/vacancy.rb

asusikov · 2019-06-26T14:28:42Z

@davydovanton Еще вопрос про dry-struct - можно ли как-то игнорировать не переданные параметры в конструктор? default только работает если ни одно значение не передано, если хоть одно, то ругается что других нет. Нашел только meta(required: false), но это не то. В документации есть про meta(omittable: true), но оно не работает и в коде не нашел ничего подобного. А то постоянно прокидывать все аттрибуты с nil как-то не айс

davydovanton · 2019-06-26T14:45:29Z

@asusikov не очень понял вопрос про драй структуру. можешь пример кода показать или тыкнуть в ПР-е?

asusikov · 2019-06-26T15:08:49Z

Я про то чтобы можно было сделать вот так

Queries::Vacancy::SearchQuery.new(remote: true)

Сейчас получаю ошибку

Dry::Struct::Error ([Queries::Vacancy::SearchQuery.new] :position_type is missing in Hash input)

И приходится делать вот так

Queries::Vacancy::SearchQuery.new(remote: true, position_type: nil, location: nil)

И если понадобится новый аттрибут, то получится везде надо будет его прокидывать. Вот можно сделать настроить схему так, чтобы он не ругался, что мы не все ключи передали?

davydovanton · 2019-06-27T10:39:04Z

в документации нашел такой пункт

Tolerance to missing keys

You can mark certain keys as optional with adding meta omittable: true to theirs types.

class User < Dry::Struct
  attribute :name, Types::Strict::String
  attribute :age,  Types::Strict::Integer.meta(omittable: true)
end

user = User.new(name: 'Jane')
# => #<User name="Jane" age=nil>
user.age
# => nil

In the example above nil violates the type constraint so be careful with :omittable.

Вот ссылка на саму документацию:
https://dry-rb.org/gems/dry-struct/recipes/

Думаю omittable + default должно хватить в этом случае

asusikov · 2019-06-28T07:37:09Z

Да, я это и пробовал - не работало. Но кажется разобрался в чем дело - у нас dry-structs версии 0.3.1 а это дело вроде с 0.5.0. Сходу у меня обновится не получилось - оставил пока как есть

asusikov · 2019-06-28T08:09:56Z

@davydovanton Замечания поправил - посмотри еще раз, пожалуйста.

asusikov · 2019-10-01T21:11:18Z

@davydovanton Что с этим PR? Есть какие-то еще замечания?

JuPlutonic · 2019-10-04T23:18:03Z

@asusikov я заметил что мердж поверх #38 делать нельзя т.к.

задисейблены все поля отвечающие за фильтрацию. apps/web/templates/vacancies/index.html.slim
когда работаешь с url запросами - работают только на один атрибут (второй query_attribute переданный ч/p GET ?query=КАКОЙ-ТОПЕРВЫЙАТРИБУТ:true&ВТОРОЙАТРИБУТ:contractor всегда игнорируется, а nil в качестве ключа обламывает контроллёр: key not found: "nil" in /apps/web/controllers/vacancies/index.rb:41, конечно же хочется чтобы уже SearchQuery омитил такие атрибуты...

JuPlutonic · 2019-10-04T23:18:42Z

apps/web/controllers/vacancies/index.rb

@@ -9,7 +9,8 @@ class Index

        include Import[
          'libs.search_query_parser',
-          operation: 'vacancies.operations.list'
+          operation: 'vacancies.operations.list',
+          search_options_mapper: 'vacancies.mappers.search_options',


не принципиально, но тут лишняя запятая

asusikov · 2019-10-09T18:29:33Z

@JuPlutonic Спасибо за замечания!

Я так понял, что сначала этот PR мержим, а потом уже делаем все в интерфейсе отдельными PR'ами. @davydovanton, рассуди.
Параметры будут передаваться не через &, а через пробел( ). Вот здесь я как раз этот момент уточнял. Про nil посмотрю

davydovanton · 2019-10-12T15:21:46Z

привет, я сейчас все посмотрю и рассужу. спасибо всем за помощь

davydovanton

в целом все выглядит разумно. я бы сначал замержил этот ПР потом второй. Жду мелких правок

davydovanton · 2019-10-12T15:22:50Z

apps/web/controllers/vacancies/index.rb

@@ -35,7 +36,9 @@ def call(params)
        private

        def search_query
-          params[:query] ? search_query_parser.call(params[:query]) : EMPTY_SEARCH_QUERY
+          query_attributes = params[:query] ? search_query_parser.call(params[:query]) : EMPTY_SEARCH_QUERY
+          initial_attributes = { remote: nil, position_type: nil, location: nil }


я бы в константу это вынес

davydovanton · 2019-10-12T15:24:26Z

apps/web/controllers/vacancies/index.rb

@@ -35,7 +36,9 @@ def call(params)
        private

        def search_query
-          params[:query] ? search_query_parser.call(params[:query]) : EMPTY_SEARCH_QUERY
+          query_attributes = params[:query] ? search_query_parser.call(params[:query]) : EMPTY_SEARCH_QUERY


а еще лучше, я бы сделал так, что квери парсер возвращал бы сразу что надо, вне зависимости от того, что туда передать, что-то в таком духе:

query_attributes = search_query_parser.call(params[:query]) query_attributes # => attributes or initial_attributes

davydovanton · 2019-10-12T15:25:21Z

lib/core/queries/vacancy.rb

+    def all_with_contact_relation(limit:, page:, search_query:)
+      query = repo.aggregate(:contact)
+                  .where(published: true, archived: false, deleted_at: nil)
+      search_query.to_h.each do |key, value|


реально оптимизировать этот кусок? что бы при nil код не выполнялся?

Не совсем понял вопрос

может быть:

if search_query ... end search_query.to_h.each do |key, value|

davydovanton · 2019-10-12T15:26:23Z

и можешь ребейз сразу сделать от мастера?

asusikov · 2019-10-13T06:41:37Z

Да, конечно. Думаю в ближайшее смогу все сделать

davydovanton · 2020-04-15T15:32:46Z

Привет, прости, что задержал ПР. я решил его замержить и уже самостоятельно доделать. Спасибо большое, если что - я могу прислать сюда ссылку на коммит с изменениями

asusikov added 5 commits June 24, 2019 21:13

Added empty search query hash to query-object

cce6147

Added filter by remote_available

63708ca

Added filters for location and position_type

6e795d1

Changed condition of running query modifier

77d8420

Added struct for vacancy search query

3bf4ac2

asusikov changed the title ~~Filters by remote_availibity, location, position_type in query object~~ [WIP] Filters by remote_availibity, location, position_type in query object Jun 25, 2019

davydovanton reviewed Jun 25, 2019

View reviewed changes

apps/web/controllers/vacancies/index.rb Outdated Show resolved Hide resolved

davydovanton reviewed Jun 25, 2019

View reviewed changes

lib/core/queries/vacancy.rb Outdated Show resolved Hide resolved

GeorgeGorbanev mentioned this pull request Jun 25, 2019

add remote filter to web/vacancies/index #38

Open

asusikov added 3 commits June 28, 2019 10:39

Moved SearchOptions to own file

2f13e85

Added SearchOptions file and mapper

ec683c3

Added spec to mapper

7963877

asusikov changed the title ~~[WIP] Filters by remote_availibity, location, position_type in query object~~ Filters by remote_availibity, location, position_type in query object Jun 28, 2019

JuPlutonic reviewed Oct 5, 2019

View reviewed changes

davydovanton requested changes Oct 12, 2019

View reviewed changes

Merge branch 'master' into 33-filters-in-query-object

df683d8

davydovanton merged commit 1ac5c74 into davydovanton:master Apr 15, 2020

davydovanton mentioned this pull request Apr 15, 2020

Fix vacancy query logic #60

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filters by remote_availibity, location, position_type in query object #39

Filters by remote_availibity, location, position_type in query object #39

asusikov commented Jun 24, 2019 •

edited

Loading

asusikov commented Jun 26, 2019

davydovanton commented Jun 26, 2019

asusikov commented Jun 26, 2019 •

edited

Loading

davydovanton commented Jun 27, 2019

asusikov commented Jun 28, 2019 •

edited

Loading

asusikov commented Jun 28, 2019

asusikov commented Oct 1, 2019

JuPlutonic commented Oct 4, 2019 •

edited

Loading

JuPlutonic Oct 4, 2019

asusikov commented Oct 9, 2019 •

edited

Loading

davydovanton commented Oct 12, 2019

davydovanton left a comment

davydovanton Oct 12, 2019

davydovanton Oct 12, 2019

davydovanton Oct 12, 2019

asusikov Oct 13, 2019

JuPlutonic Oct 13, 2019 •

edited

Loading

davydovanton commented Oct 12, 2019

asusikov commented Oct 13, 2019

davydovanton commented Apr 15, 2020

Filters by remote_availibity, location, position_type in query object #39

Filters by remote_availibity, location, position_type in query object #39

Conversation

asusikov commented Jun 24, 2019 • edited Loading

What this PR do?

asusikov commented Jun 26, 2019

davydovanton commented Jun 26, 2019

asusikov commented Jun 26, 2019 • edited Loading

davydovanton commented Jun 27, 2019

Tolerance to missing keys

asusikov commented Jun 28, 2019 • edited Loading

asusikov commented Jun 28, 2019

asusikov commented Oct 1, 2019

JuPlutonic commented Oct 4, 2019 • edited Loading

JuPlutonic Oct 4, 2019

Choose a reason for hiding this comment

asusikov commented Oct 9, 2019 • edited Loading

davydovanton commented Oct 12, 2019

davydovanton left a comment

Choose a reason for hiding this comment

davydovanton Oct 12, 2019

Choose a reason for hiding this comment

davydovanton Oct 12, 2019

Choose a reason for hiding this comment

davydovanton Oct 12, 2019

Choose a reason for hiding this comment

asusikov Oct 13, 2019

Choose a reason for hiding this comment

JuPlutonic Oct 13, 2019 • edited Loading

Choose a reason for hiding this comment

davydovanton commented Oct 12, 2019

asusikov commented Oct 13, 2019

davydovanton commented Apr 15, 2020

asusikov commented Jun 24, 2019 •

edited

Loading

asusikov commented Jun 26, 2019 •

edited

Loading

asusikov commented Jun 28, 2019 •

edited

Loading

JuPlutonic commented Oct 4, 2019 •

edited

Loading

asusikov commented Oct 9, 2019 •

edited

Loading

JuPlutonic Oct 13, 2019 •

edited

Loading