Automation rule content matching and thread renaming action v1 #445
Conversation
|
Este je tam nejaka chyba v testoch, to zatial pls ignoruj, fixnem |
|
@jsuchal za mna hotovo, pozri pls |
app/models/automation/action.rb
Outdated
| else | ||
| thing | ||
| end | ||
| new_value = value.gsub("${title}", object.title) |
There was a problem hiding this comment.
Tato notacia ide odkial? Som skor za handlebars styl {{title}}
| @@ -0,0 +1,34 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| module ContentMatchingOperations | |||
There was a problem hiding this comment.
Imho mixiny davaju zmysel ked to pojde do viacerych tried, toto mi pride, ze cela tato logika by mala byt v action samotnej.
There was a problem hiding this comment.
Zaviedol som to prave preto, ze to ide do dvoch tried - message_object a nested_message_object. Mam to teda stahovat do action?
There was a problem hiding this comment.
No praveze mne pride ze by to malo byt len v action a prinajlepsom message_object alebo nested_message_object budu mat nejaku malu metodu ktora bude vytahovat obsah pre toto. nedava mi zmysel uplne rovnake metody tlacit do dvoch tried, to smrdi.
config/locales/sk.yml
Outdated
| boxes: | ||
| sync_all_requested: "Sťahovanie nových správ bolo spustené." | ||
| message_created: "Nová správa" | ||
| "Automation::ContainsCondition": "obsahuje" | ||
| "Automation::AttachmentContentCondition": "obsahuje" |
There was a problem hiding this comment.
Tie conditions by mali byt nazvate tak, ze vieme co sa bude diat. Cize skor Automation::AttachmentContentContainsCondition
There was a problem hiding this comment.
Mozno by som tu dal príloha obsahuje ?
There was a problem hiding this comment.
Premenovane na Automation::AttachmentContentContainsCondition
Aj ja som najprv isiel, ze priloha obsahuje, ale kvoli GUI a jeho skladaniu treba takto:
| pdf.pages.each do |page| | ||
| if (last_page_text + " " + page.text).gsub("\n", " ").match?(value) | ||
| io.close | ||
| return true | ||
| end | ||
|
|
||
| last_page_text = page.text |
There was a problem hiding this comment.
Tejto logike uplne nerozumiem. jednak toto bude nachylne na whitespace a diakritiku a dvak preco matchujeme len predchadzajucu page a tu nasledujucu? Co ak je text cez 3 strany? (protipriklad)
There was a problem hiding this comment.
Tiez sa mi to velmi nepaci, ale skor teda daj pls nejaky tip, ako na to. Kedze som nic lepsie nevymyslel. Mam hladat nejake riesenia s ignorovanim whitespace a diakritiky?
Co sa tyka 3 stran, tak to by som povazoval za taky extrem, ze to asi neriesime. Zatialco rozseknutie hladaneho textu na 2 strany asi moze celkom bezne nastat.
There was a problem hiding this comment.
Ja by som to asi cele hodil do jedneho stringu a matchoval tam.
test/fixtures/automation/actions.yml
Outdated
| six: | ||
| automation_rule: five | ||
| type: Automation::ChangeMessageThreadTitleAction | ||
| value: "New title - ${title}" |
There was a problem hiding this comment.
Ok, rozumiem tomu teda tak, ze do tohto ${title} sa vlastne vklada nazov vlakna a mozem si nastavit na urovni akcie, ze ako sa to premenuje. Asi good enough na teraz.
(Ako nejake rozsirenie si viem predstavit, ze budes matchovat regexp a vytiahnes z neho nieco co tu vlozis.)
| @@ -20,8 +20,12 @@ | |||
| <p class="flex-grow-0 flex-shrink-0 text-base text-left text-gray-500"> | |||
| <% @automation_rule.actions.each_with_index do |action, index| %> | |||
| <span class="flex-grow-0 flex-shrink-0 text-base text-left text-gray-500"><%= t action.type %></span> | |||
| <span class="flex-grow-0 flex-shrink-0 text-base font-medium text-left text-gray-500"><%= action.action_object.name %></span> | |||
| <%if index < @automation_rule.actions.length - 1%><%= ', '%><% end %> | |||
| <% if action.type == 'Automation::AddMessageThreadTagAction' %> | |||
app/models/automation/action.rb
Outdated
| @@ -22,13 +22,18 @@ class Action < ApplicationRecord | |||
| def tag_list | |||
| automation_rule.tenant.tags.pluck(:name, :id) | |||
| end | |||
|
|
|||
| def object_based? | |||
There was a problem hiding this comment.
Toto sa robit tak, ze tu metodu implementujes v actions a tam vratis true alebo false.
app/models/automation/condition.rb
Outdated
| pdf.pages.each do |page| | ||
| pdf_string += page.text | ||
| last_page_text = page.text | ||
| end |
There was a problem hiding this comment.
Tu by som to cele nacital. pdf_string = pdf.pages.map(&:text).join(' ')
There was a problem hiding this comment.
@celuchmarek medzicasom stihol nejake veci co ti koliduju porobit, skus spravit merge/rebase voci main.
|
@celuchmarek ty si sa nedavno chytal actions, vies kuknut ci tu nevidis nieco podozrive? Za mna to uz vyzera celkom ok. |
|
Este tam je nejaky problem na frontende. Ale @celuchmarek samozrejme mozes pozriet, snad to uz bude lahky fix. |
|
Tie veci, čo sa dotýkajú mojich zmien, vyzerajú podľa mňa ok. |
No description provided.