BillTechPL / lms-billtech-plugin

Other
6 stars 7 forks source link

parę usprawnień związanych z akcjami "odblokującymi" klienta #98

Closed chilek closed 2 years ago

chilek commented 2 years ago

Obecnie

https://github.com/BillTechPL/lms-billtech-plugin/blob/51440eefdf1857796c866339a45c66edfaefa89e/BillTechPaymentsUpdater.php#L200-L208

wtyczka na własną rękę odblokowuje komputer klienta, a to wprowadza konflikt z natywnie używanymi w LMS mechanizmami odblokowywania klienta, gdzie taka procedura składa się z wielu wykonywanych akcji. Aktualnie stosowanym przez nas rozwiązaniem w paru wdrożeniach jest użycie cuttoff.limit o wartości 50000 (swoją drogą to ustawienie też powinno raczej znajdować się w sekcji billtech dla porządku - być może pod inną nazwą - np. cutoff_limit). Nadanie tak dużej wartości rozwiązuje konflikt mechanizmów odblokowujących klienta pochodzących z wtyczki i CORE (w CORE używamy bin/lms-notify.php i kanału unblock - ten kanał powiadomień odpowiada tak naprawdę za realizację wskazanych akcji odblokowujących).

Fajnie byłoby konfiguracją wtyczki BillTech mieć możliwość wskazania zewnętrznego polecenia systemowego wskazującego skrypt do wykonania złożonego odblokowania klienta z obsługą jakiegoś magicznego symbolu %cid% zastępowanego identyfikatorem klienta. To by umożliwiło wykonanie złożonych zadań odblokowujący dla pojedynczego klienta (obecnie próby odblokowania wykonujemy cyklicznie, co parę godzin próbując wszystkich klientów, a to wiadomo, że obciąża znacząco przede wszystkim bazę danych).

chilek commented 2 years ago

Alternatywą w stosunku do wykonywania wskazanego, zewnętrznego polecenia mogłoby być jakieś przekazywanie komunikatów przez jakąś "magistralę". Wówczas wtyczka BillTech wstawiałaby komunikat o potrzebie odblokowania klienta do kolejki, a jakiś skrypt backendowy zaglądałby cyklicznie do takiej kolejki oczekujących zleceń/żądań. Tylko, że o ile to byłoby bardziej eleganckie, to pewnie wymagałoby poświęcenia znacznie więcej czasu na zaprojektowanie czegoś takiego.

chilek commented 2 years ago

Dziś pojawił się inny pomysł na to - wyłączenie mechanizmu odblokowującego w Wasze wtyczce, a zamiast tego, co parę minut odszukiwanie ostatnich potwierdzonych przez Wasze systemy wpłat. Czy gdzieś w bazie danych w Waszych tabelach znajdziemy taką datę potwierdzenia operacji z dokładnością do sekundy?

pablos-billtech commented 2 years ago

@chilek Czy mógłbym prosić o opisanie etapów mechanizmu, który skutkuje odblokowaniem klienta?

pablos-billtech commented 2 years ago

Dziś pojawił się inny pomysł na to - wyłączenie mechanizmu odblokowującego w Wasze wtyczce, a zamiast tego, co parę minut odszukiwanie ostatnich potwierdzonych przez Wasze systemy wpłat. Czy gdzieś w bazie danych w Waszych tabelach znajdziemy taką datę potwierdzenia operacji z dokładnością do sekundy?

W tabeli billtech_payments jest kolumna cdate, która przyjmuje wartość stworzenia wpłaty tymczasowej w LMS, czyli jednoznacznie wskazuje, że dany klient opłacił zobowiązanie.

Zastanawiam się tylko, czy lepiej nie mieć tego w jednym miejscu i żeby odpalało się praktycznie od razu, tak jak myśleliśmy, że to się dzieje do tej pory w wyżej wskazanym fragmencie kodu. Chyba że problemem przy tym podejściu są różnice w implementacji etapów mechanizmu odblokowującego klienta wśród różnych LMSów.

chilek commented 2 years ago

@pablos-billtech myślę, że nie warto opisywać mechanizmu, bo co firma to on się różni, ale z grubsza wystarczy uruchomienie skryptu:

${LMSDIR}/bin/lms-notify.php -c unblock -t debtors -s odblokowania --actions node-access,customer-status,customer-group(zablokowani_windykacja),node-group(zablokowani_windykacja),jambox-locks

przy założeniu, że mamy ustawienia:

[odblokowania]
customergroups = "!bez_powiadomien"
debtors_days = "14"
debtors_limit = "-10"

Jest taki kawałek kodu: https://github.com/BillTechPL/lms-billtech-plugin/blob/51440eefdf1857796c866339a45c66edfaefa89e/BillTechPaymentsUpdater.php#L132-L134

o czy cdate jest datą ostatecznego potwierdzenia płatności, czy tylko datą rozpoczęcia płatności?

chilek commented 2 years ago

Zastanawiam się tylko, czy lepiej nie mieć tego w jednym miejscu i żeby odpalało się praktycznie od razu, tak jak myśleliśmy, że to się dzieje do tej pory w wyżej wskazanym fragmencie kodu. Chyba że problemem przy tym podejściu są różnice w implementacji etapów mechanizmu odblokowującego klienta wśród różnych LMSów.

Dziś doszedłem do wniosku, że jednak nie warto tego modyfikować w Waszej wtyczce, tylko przyjąć założenie, że u Was minimum zmian (albo w ogóle), a my sobie odczytamy w różnych instalacjach odpowiednim, prostym zapytaniem SQL (ewentualnie wtyczka mogłaby mieć jakąś metodę zwracającą listę potwierdzonych płatności w ciągu ostatnich X sekund - żeby uniezaleźnić się od zmian w schemacie Waszych tabel).

chilek commented 2 years ago

tylko, co tak naprawdę oznacza billtech_payments.closed? Widziałem w kodzie, że może być 0 lub 1.

chilek commented 2 years ago

A insert z komentarza

https://github.com/BillTechPL/lms-billtech-plugin/issues/98#issuecomment-1219506849

ustawia closed na 0. A potem może zmienić się ta wartość na 1.

chilek commented 2 years ago

Z tego, co widzę w kodzie to closed = 0 - oznacza, że prawdopodobnie opłacono, a 1 że wpłata nie została ostatecznie potwierdzona, a tym samym została anulowana?

chilek commented 2 years ago

więc chyba w sumie, tak jak piszesz, możemy założyć, że wpłata została wykonana (choć jest niepotwierdzona) i tym samym szukać rekordów w blltech_payments z closed = 0 i cdate >= ?NOW - X sekund i tych klientów próbować odblokować. Gdy closed zmieni się na 1 (anulowano) nasz automat blokujący jeszcze raz zablokuje klienta.

pablos-billtech commented 2 years ago

czy cdate jest datą ostatecznego potwierdzenia płatności, czy tylko datą rozpoczęcia płatności?

cdate jest datą powstania wpłaty tymczasowej w LMS, czyli ostateczną datą potwierdzenia płatności, na które się można zapiąć w celu odblokowania.

tylko, co tak naprawdę oznacza billtech_payments.closed? Widziałem w kodzie, że może być 0 lub 1.

Rekord w tabeli billtech_payments to tak zwana wpłata tymczasowa. Powstaje ona jako potwierdzenie dokonania płatności przez klienta i pozwala zarówno administratorowi i klientowi na zauważenie w LMS, że płatność została wykonana. Wpłaty tymczasowe są widoczne w LMS jako "Wpłata online za: <tytuł płatności>". Wpłata tymczasowa zostaje docelowo zastąpiona wpłatą z banku, która jest importowana przez mechanizm cashimport, stąd nazwa tymczasowa. Gdy tak się stanie to closed przyjmuje wartość 1, żeby to pokazać. W LMSie widać to poprzez wyszarzenie się odpowiedniego rekordu w widoku Płatności Billtech.

Można też o tym doczytać tutaj

chilek commented 2 years ago

@pablos-billtech zostaje w takim razie pytanie: czy macie jakąś metodę w którejś klasie wtyczki, która pozwoliłaby odczytać gotową listę wpłat z billtech_payments z closed = 0 i cdate >= ?NOW - X sekund?

chilek commented 2 years ago

Matwi mnie to, że rekord w billtech_payments jest wstawiany z cdate równym: https://github.com/BillTechPL/lms-billtech-plugin/blob/51440eefdf1857796c866339a45c66edfaefa89e/BillTechPaymentsUpdater.php#L106 co oznacza, że teoretycznie data może być dość odległa, np. sprzed godziny. Dałoby radę dorobić w tej tabeli datę faktycznego wstawienia rekordu do bazy (wiem możemy we własnym skrypcie sprawdzać jakie ID płatności BillTech ostatnio przetworzyliśmy i stratować od ID+1 z weryfikacją czy odblokowywac)?

chilek commented 2 years ago

możemy we własnym skrypcie sprawdzać jakie ID płatności BillTech ostatnio przetworzyliśmy i stratować od ID+1 z weryfikacją czy odblokowywac

Chyba trzeba w takim kierunku pójść? @pablos-billtech są jakieś przeciwskania ku temu?

chilek commented 2 years ago

Hmm billtech_payments nie ma jednak ID, tylko coś takiego jak cashid lub reference_number. W takim razie kolejna wątpliwość - jak one są wyliczane - narastająco?

pablos-billtech commented 2 years ago

Hmm billtech_payments nie ma jednak ID, tylko coś takiego jak cashid lub reference_number. W takim razie kolejna wątpliwość - jak one są wyliczane - narastająco?

Z tego co patrzę u siebie w bazie, to mają tylko nie widać tego w "INSERT INTO billtech_payments ..." bo wartość się autoinkrementuje.

chilek commented 2 years ago

Sam sobie odpowiadam - cashid w billtech_payments wskazuje rekord w cash jeśli ma wartość nie NULL. To jest chyba pewnego rodzaju powiązanie bez jawnego klucza obcego/referencji?

chilek commented 2 years ago

Z tego co patrzę u siebie w bazie, to mają tylko nie widać tego w "INSERT INTO billtech_payments ..." bo wartość się autoinkrementuje.

Faktycznie - jest pole id w bazie jako serial/sequence! Wystarczy!

chilek commented 2 years ago

Dzięki za pomoc!

pablos-billtech commented 2 years ago

możemy we własnym skrypcie sprawdzać jakie ID płatności BillTech ostatnio przetworzyliśmy i stratować od ID+1 z weryfikacją czy odblokowywac Chyba trzeba w takim kierunku pójść? @pablos-billtech są jakieś przeciwskania ku temu?

Moim zdaniem nie ma przeciwskazań!

chilek commented 2 years ago

Docelowo więc lista klientów będzie wyglądać następująco:

lms=> SELECT DISTINCT customerid FROM billtech_payments WHERE closed = 0 AND id >= 96;
 customerid 
------------
     200118
     212845
     213571
     217360
(4 wiersze)

zakładając, że dla warunki ID podajemy ID ostatnio przetworzonego rekordu z billtech_payments.

chilek commented 2 years ago

... i tylko dla takich klientów znalezionych co 5 minut wykonany pełne procedury odblokowania.

pablos-billtech commented 2 years ago

To spowoduje odblokowanie usługi nawet dla kogoś, kto zapłacił 100zł, a był winien 1000zł. Z tego powodu we wtyczce my obliczamy czy po potwierdzeniu płatności nowe saldo po przekroczeniu terminu płatności jest mniejsze od cutoff.limit.

if ($extended_balance >= $cutoff_limit) {
            $LMS->NodeSetU($customerid, TRUE);
...
pablos-billtech commented 2 years ago

Myślę, że możemy wystawić Wam taką funkcję, która będzie zwracać boolean w zależności, czy naszym zdaniem można, czy nie można odblokować na podstawie powyższego kryterium.

chilek commented 2 years ago

@pablos-billtech i moglibyśmy podawać opcjonalnie ID ostatniego rekordu z billtech_payments, który ostatnio przetworzyliśmy? (gdybyśmy nie podali tego ID to wtyczka Wasza na własną rękę przy każdym wywołaniu tej metody aktualizowała jakieś własne "wewnętrzne" ustawienie konf.).

chilek commented 2 years ago

To spowoduje odblokowanie usługi nawet dla kogoś, kto zapłacił 100zł, a był winien 1000zł. Z tego powodu we wtyczce my obliczamy czy po potwierdzeniu płatności nowe saldo po przekroczeniu terminu płatności jest mniejsze od cutoff.limit.

Nasz automat odblokowujący z kombajnu lms-notify.php i tak liczy saldo dla każdego klienta.

Taka ciekawostka:

[lmsplus.mynet.com.pl bin]# time /var/www/html/lms/bin/lms-notify.php -c unblock -s odblokowania -t debtors --actions 'node-access,customer-status,customer-group(zablokowani_windykacja),node-group(zablokowani_windykacja),jambox-locks' -d
lms-notify.php
(C) 2001-2021 LMS Developers
Using file /etc/lms/lms.ini as config.
[unblock/node-access] Customer: #200118, Node: #200996
[unblock/customer-status] CustomerID: #200118
[unblock/customer-group] Customer: #200118, CustomerGroup: #200157
[unblock/node-group] Customer: #200118, Node: #200996

real    0m27,612s
user    0m4,236s
sys 0m0,328s
[lmsplus.mynet.com.pl bin]# time /var/www/html/lms/bin/lms-notify.php -c unblock -s odblokowania -t debtors --actions 'node-access,customer-status,customer-group(zablokowani_windykacja),node-group(zablokowani_windykacja),jambox-locks' -d --customerid 200118
lms-notify.php
(C) 2001-2021 LMS Developers
Using file /etc/lms/lms.ini as config.
[unblock/node-access] Customer: #200118, Node: #200996
[unblock/customer-status] CustomerID: #200118
[unblock/customer-group] Customer: #200118, CustomerGroup: #200157
[unblock/node-group] Customer: #200118, Node: #200996

real    0m0,968s
user    0m0,405s
sys 0m0,097s
[lmsplus.mynet.com.pl bin]# 

Dlatego zależało mi, żeby co 5 minut nie piłować po tysiącach klientów, żeby ustalić których pojedyncznych odblokować.

chilek commented 2 years ago

Czas pierwszy (dla wszystkich klientów) potrafi być znacznie dłuży, a dla pojedynczego klienta praktycznie 1 sekunda.

pablos-billtech commented 2 years ago

@pablos-billtech i moglibyśmy podawać opcjonalnie ID ostatniego rekordu z billtech_payments, który ostatnio przetworzyliśmy? (gdybyśmy nie podali tego ID to wtyczka Wasza na własną rękę przy każdym wywołaniu tej metody aktualizowała jakieś własne "wewnętrzne" ustawienie konf.).

Nie do końca rozumiem o jakie wewnętrze ustawienia konfiguracyjne ma chodzić. Fragment wtyczki związany z cutoffem nie aktualizuje po stronie wtyczki żadnych danych w tabelach związanych z wtyczką ani tym bardziej zmiennych konfiguracyjnych, więc nie powinno być problemu.

Funkcja o której myślę by tylko wyliczała czy dany użytkownik lub ewentualnie grupa(/wszyscy) użytkowników mogą zostać odblokowani. Przy czym opcja wszystkich użytkowników to więcej roboty.

pablos-billtech commented 2 years ago

Ta funkcja by wyglądała mniej więcej tak:


public function checkIfUserShouldBeCutoff($customerId) {
$extend_deadline = ConfigHelper::getConfig('cutoff.extend_deadline', 7);

$extended_balance = $this->getCustomerDueBalance($customerid, $extend_deadline);
$balance = $this->getCustomerDueBalance($customerid, 0);

$cutoff_limit = ConfigHelper::getConfig('cutoff.limit', 0);

return $extended_balance >= $cutoff_limit;
}
chilek commented 2 years ago

Nie do końca rozumiem o jakie wewnętrze ustawienia konfiguracyjne ma chodzić. Fragment wtyczki związany z cutoffem nie aktualizuje po stronie wtyczki żadnych danych w tabelach związanych z wtyczką ani tym bardziej zmiennych konfiguracyjnych, więc nie powinno być problemu.

Chodzi o to, że to wtyczka mogłaby przechowywać ID ostatnio przetworzonego rekordu z billtech_payments. Wywołanie jakieś nowej funkcji z wtyczki powodowałoby odczyt rekordów z ID > ostatnio przetworzony ID i aktualizowało ostatnio przetworzone ID, a to mogłoby być pamiętane w uiconfig - dlatego nazwałem ustawienie wewnętrzne, zarezerwowane dla kodu wtyczku, a nie wykorzystania użytkowników LMS-a. To miałem na myśli przez wewnętrzne ustawienie.

chilek commented 2 years ago

Ta funkcja by wyglądała mniej więcej tak:

public function checkIfUserShouldBeCutoff($customerId) {
$extend_deadline = ConfigHelper::getConfig('cutoff.extend_deadline', 7);

$extended_balance = $this->getCustomerDueBalance($customerid, $extend_deadline);
$balance = $this->getCustomerDueBalance($customerid, 0);

$cutoff_limit = ConfigHelper::getConfig('cutoff.limit', 0);

return $extended_balance >= $cutoff_limit;
}

W takim razie nie o to chodzi - bo wyznacza klientów, którzy wg wtyczki mogą zostać odblokowani?

pablos-billtech commented 2 years ago

Tak byłoby najłatwiej i w zasadzie wymagałoby tylko drobnego refactoru. Mogę też przygotować taką funkcję dla z góry narzuconej grupy użytkowników (zamiast $customerId byłoby $customerIds i wtedy dalej to samo w pętli) lub dla wszystkich zablokowanych użytkowników w bazie.

chilek commented 2 years ago

Naprędce skleciłem szkielet tego, o co chodzi (skrypt bash):

SECTION="odblokowania"

# MyNET
PERMANENT_OTHER_PARAMS="--actions node-access,customer-status,customer-group(zablokowani_windykacja),node-group(zablokowani_windykacja),jambox-locks"
export INI_FILE="/etc/lms/lms.ini"
export COMPANY_NAME="MyNET"

last_id=$(psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "SELECT value FROM uiconfig WHERE section = '${SECTION}' AND var = 'billtech_payment_last_id';")
if [ -z "${last_id}" ]; then
    psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "INSERT INTO uiconfig (section, var, value) VALUES ('${SECTION}', 'billtech_payment_last_id', '0');"
    last_id="0"
fi
declare -A customers
for record in $(psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "SELECT id, customerid FROM billtech_payments WHERE closed = 0 AND id > ${last_id} ORDER BY id;"); do
    id=$(echo ${record} |cut -d"|" -f1)
    customerid=$(echo ${record} |cut -d"|" -f2)
    if [ "${id}" -gt "${last_id}" ]; then
        last_id=${id}
    fi
    customers[${customerid}]=${customerid}
done
for customerid in ${customers[*]}; do
    export OTHER_PARAMS="${PERMANENT_OTHER_PARAMS} --customerid ${customerid}"
    /opt/chilan/notifies.sh unblock ${SECTION} debtors ${PARAM}
done
psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "UPDATE uiconfig SET value = '${last_id}' WHERE section = '${SECTION}' AND var = 'billtech_payment_last_id';" &>/dev/null
unset customers

# Interserv
LMSDB_NAME="lms-netcontrol"
PERMANENT_OTHER_PARAMS="--actions node-access,customer-status,customer-group(zablokowani_windykacja),node-group(zablokowani_windykacja)"
export INI_FILE="/etc/lms/lms-netcontrol.ini"
export COMPANY_NAME="Interserv"

last_id=$(psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "SELECT value FROM uiconfig WHERE section = '${SECTION}' AND var = 'billtech_payment_last_id';")
if [ -z "${last_id}" ]; then
    psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "INSERT INTO uiconfig (section, var, value) VALUES ('${SECTION}', 'billtech_payment_last_id', '0');"
    last_id="0"
fi
declare -A customers
for record in $(psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "SELECT id, customerid FROM billtech_payments WHERE closed = 0 AND id > ${last_id} ORDER BY id;"); do
    id=$(echo ${record} |cut -d"|" -f1)
    customerid=$(echo ${record} |cut -d"|" -f2)
    if [ "${id}" -gt "${last_id}" ]; then
        last_id=${id}
    fi
    customers[${customerid}]=${customerid}
done
for customerid in ${customers[*]}; do
    export OTHER_PARAMS="${PERMANENT_OTHER_PARAMS} --customerid ${customerid}"
    /opt/chilan/notifies.sh unblock ${SECTION} debtors ${PARAM}
done
psql -At -U ${LMSDB_USER} ${LMSDB_NAME} -c "UPDATE uiconfig SET value = '${last_id}' WHERE section = '${SECTION}' AND var = 'billtech_payment_last_id';" &>/dev/null
unset customers
chilek commented 2 years ago

Można to śmiało co 5 minut uruchamiać, bo gdy nie ma płatności to:

[lmsplus.mynet.com.pl chilan]# time ./windykacja-unblock-billtech.sh

real    0m0,205s
user    0m0,042s
sys 0m0,077s
[lmsplus.mynet.com.pl chilan]# 
pablos-billtech commented 2 years ago

Ja myślę o takim właśnie skrypcie, który zamiast odpytywania bazy: "SELECT id, customerid FROM billtech_payments WHERE closed = 0 AND id > ${last_id} ORDER BY id;" ponieważ to może spowodować niepoprawne odblokowanie klienta, który zapłacił część zobowiązania, ale nie całe.

Zamiast tego odpytywał by dodaną specjalnie do wtyczki funkcję w PHP podobnej do tej z moich poprzednich komentarzy wyżej, która by wprost wskazała jakich użytkowników należy odblokować. Wtedy nie trzeba dodawać nowych zmiennych konfiguracyjnych, tylko wystarczy odpytać wtyczki, który klient jest gotowy do oblokowania. Takie rozwiązanie byłoby też odporne na zmiany schematów tabel.

Pytanie, czy czegoś nie widzę?

chilek commented 2 years ago

ponieważ to może spowodować niepoprawne odblokowanie klienta, który zapłacił część zobowiązania, ale nie całe.

Ale rekordy w billtech_payments zawierają pełne kwoty płatności? Jeśli tak to nie będzie w naszym przypadku tego problemu, gdyż my i tak przy próbie odblokowania liczymy pełne saldo klienta.

Zamiast tego odpytywał by dodaną specjalnie do wtyczki funkcję w PHP podobnej do tej z moich poprzednich komentarzy wyżej, która by wprost wskazała jakich użytkowników należy odblokować. Wtedy nie trzeba dodawać nowych zmiennych konfiguracyjnych, tylko wystarczy odpytać wtyczki, który klient jest gotowy do oblokowania. Takie rozwiązanie byłoby też odporne na zmiany schematów tabel.

To będzie zapewne robione w inny sposób niż robi lms-notify.php z -t debtors. Można oczywiście zrobić identycznie jak pozwala to lms-notify.php, ale po co duplikować kod?

Z naszej strony byłoby bardziej istotne uzyskiwanie klientów wprost z nieprzetworzonych płatności (z podbijaniem automatycznym ID ostatnio przetworzonego rekordu z billtech_payments). bin/lms-notify.php i tak wyliczy saldo zgodnie z nieraz złożonymi zasadami i odblokuje (lub nie) zależnie od tego klienta. Tak właściwie to obecnie tak to już działa w MyNET i Interserv.

chilek commented 2 years ago

@pablos-billtech moim zdaniem nie warto, żebyście martwili się jaki będzie sposób odblokowania i co konkretnie będzie robione. Jedynie wystarczy, żebyście typowali ewentualnie klientów do odblokowania (co robi mój wczorajszy skrypt bash).

pablos-billtech commented 2 years ago

Ale rekordy w billtech_payments zawierają pełne kwoty płatności? Jeśli tak to nie będzie w naszym przypadku tego problemu, gdyż my i tak przy próbie odblokowania liczymy pełne saldo klienta.

Tak, zawierają pełne kwoty płatności.

Czyli podsumowując, coś zostało do zrobienia po naszej stronie, czy możemy zamykać to issue?

chilek commented 2 years ago

Moim zdaniem nic, chyba, że masz ochotę na zrobienie czegoś, co będzie odpowiadać skryptowi bash powyżej, a będzie pojedyncznym wywołaniem metody z Waszej wtyczki.