Automatické postovanie do fóra. #7
Conversation
| @@ -0,0 +1,18 @@ | |||
| require 'discourse_api' | |||
|
|
|||
| class DiscourseClient < DiscourseApi::Client | |||
There was a problem hiding this comment.
Chcel som aby bol Discourse notifier zavisly iba na interface injektnuteho klienta. A tento koncept narusal fakt, ze musim chytat custom exception class z discourse gemu. Tak som urobil nas discource klient, ktory momentalne len wrapne ten z gemu a takisto prelozi ich exception na exception nasho klienta.
A tu exception potrebujem na to, aby som vedel odfiltrovat discourse api errory od ostatnych.
|
|
||
| module Uvobot | ||
| module Notifications | ||
| class Base |
There was a problem hiding this comment.
AbstractNotifier alebo rovno Notifier
| @notifier.no_announcements_found | ||
| end | ||
| end | ||
| require_relative './uvobot/worker' |
There was a problem hiding this comment.
Toto ./uvobot/worker nepoznam, ja pouzivam require_relative 'uvobot/worker'
There was a problem hiding this comment.
Nechapem. Nepaci sa ti ze tam volam cely namespace naraz, alebo ze tam je bodka v ceste?
There was a problem hiding this comment.
Ta bodka, podla mna to je zbytocne.
|
Ak ta to prestane bavit tak povedz. Nech nemame z toho blby pocit obaja. |
| @@ -0,0 +1,8 @@ | |||
| require_relative 'notifications/notifier' | |||
There was a problem hiding this comment.
Cely tento subor notifiactions.rb podla mna moze ist prec.
There was a problem hiding this comment.
Moze. Pouzivam to na skupinovy require, ked rozpisem jednotlive notifiers nebude ho treba.
| def announcement_to_topic(announcement) | ||
| detail = @scraper.get_announcement_detail(announcement[:link][:href]) | ||
|
|
||
| { |
There was a problem hiding this comment.
Tu mi chyba to osetrenie, ze ked je detail nil = nepodarilo sa ho extrahovat tak to vytvori topic s linkom ale default spravou, ze k tomu obstarku sa nepodarilo nic viac vytiahnut.
There was a problem hiding this comment.
Je to tam ponate tak, ze metoda detail_message da namiesto ceny spravu o nepodarenej extrakcii. A link na detail/zdroj obstaravania, maju hned pod tym.
To fakt chces tvorit zvlast topic s oznamom ze sa nepodarilo vytiahnut cenu pre kazde obstaravanie co ma iny format?
There was a problem hiding this comment.
Inak, ak sa nepodari vytiahnut ziadne data tak by to malo vytvorit stale topic. Ak sa nepodari len cenu tak je to v pohode tak ako to je teraz.
EDIT: Ah! uz to vidim. Dobre. Nic.
|
Z coho presne mas blby pocit? Nebavi ta nekonecny code review? :) |
| detail = {} | ||
| h_doc = doc(html) | ||
| amount_node = h_doc.xpath('//div[text()="Hodnota "]').css('span').first | ||
| fail Uvobot::ParsingError, 'Amount node not found.' if amount_node.nil? |
There was a problem hiding this comment.
Toto si stale myslim, ze je v kontexte parsovania uplne bezna situacia, cize by sa normalny control flow nemal robit cez Exception. nech to vrati nil. Aj tak sa to odchytava hned vyssie. Neni to exception co chytas cez "4 poschodia".
|
Mna hej, len ci nemas pocit, ze uz moc rypem. |
|
Beriem to ako mentoring. Plus to má zjavne pozitívny dopad na kód :). |
| @@ -0,0 +1,55 @@ | |||
| require_relative 'notifier' | |||
| require_relative '../uvo_scraper' | |||
|
Ked toto dobojujeme, mozem sa pozriet aj na to "nenotifikovanie" cez vikend. |
| UvoScraper.new(UvoParser) | ||
| notifiers = [ | ||
| Uvobot::Notifications::SlackNotifier.new(ENV.fetch('UVOBOT_SLACK_WEBHOOK')), | ||
| Uvobot::Notifications::DiscourseNotifier.new(discourse_client, 'Štátne projekty', Uvobot::UvoScraper.new) |
There was a problem hiding this comment.
Este mi toto vytiahni do env DISCOURSE_TARGET_CATEGORY a moze byt.
Automatické postovanie do fóra, closes #2
V code review už môžeme pokračovať priamo v PR. Momentálne je to urobené tak, aby boli notifiery plne kompatibilné reps. zameniteľné. Teda implementujú ten istý interface, ktorý je podchytený v base triede.