-
Notifications
You must be signed in to change notification settings - Fork 136
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
enhancement: introduce new liabilities suspention methods #2474
base: master
Are you sure you want to change the base?
enhancement: introduce new liabilities suspention methods #2474
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Ciekawe, jaki wpływ na wydajność będzie miało użycie tak mocno rozbudowanych, nowo dodawanych widoków.
- Czy dla wszystkich zmian uwzględniających wprowadzenie nowego sposobu zawieszeń robiłeś jakieś podstawowe, chociaż minimalne testy, Czy raczej część zmian jest czysto teoretyczna?
- Od strony merytorycznej zmian ciężko mi się wypowiadać nie wykonując żadnych testów. Podejrzewam, że początek następnego miesiąca i generowanie masowe zobowiązań pokaże, czy działa chociaż to, co dotąd działało - najpierw zaktualizuję instalacje u nas i wykonam lms-payments.php w trybie
--test
przez generowaniem produkcyjnym. Jeśli zadziała to, co dotąd działało, to już będzie połowa sukcesu.
bin/lms-payments.php
Outdated
@@ -473,45 +500,81 @@ function get_period($period) | |||
LEFT JOIN promotionschemas ps ON ps.id = a.promotionschemaid | |||
LEFT JOIN promotions p ON p.id = ps.promotionid | |||
LEFT JOIN ( | |||
SELECT | |||
SELECT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uwaga ogólna - git i github wskazują, że spacja na końcu wiersza jest podejrzana, więc nie wprowadzajmy tego x powrotem, co już było usuwane cierpliwie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poprawiłem.
bin/lms-payments.php
Outdated
FROM assignments a | ||
JOIN customers c ON a.customerid = c.id | ||
JOIN customers c ON (a.customerid = c.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Z nawiasów od jakiegoś czasu rezygnujemy, bo niczemu one nie służą. Zasada uniwersalna dla warunków w * JOIN.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poprawiłem.
bin/lms-payments.php
Outdated
" . ($billing_invoice_separate_fractions ? ' COALESCE(voipcost.call_count, 0) AS call_count, COALESCE(voipcost.call_fraction, \'\') AS call_fraction , ' : '') . " | ||
voipphones.phones, | ||
(CASE WHEN EXISTS (SELECT 1 FROM customerconsents cc WHERE cc.customerid = c.id AND cc.type IN ?) THEN 1 ELSE 0 END) AS billingconsent | ||
FROM assignments a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Źle wygląda brak wcięcia dla kolejnych wierszy zapytania SQL względem pierwszego wiersza polecenia PHP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poprawiłem. Mam nadzieję, że o to Ci chodziło.
(CASE WHEN suspensions.taxlabel IS NOT NULL | ||
THEN suspensions.taxlabel | ||
ELSE suspensions_all.taxlabel | ||
END) AS suspension_taxlabel, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
W takich miejsach nie móżna się posłużyć w sprytny sposób funkcją SQL COALESCE()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tu akurat powinno być:
(CASE WHEN **suspensions.suspension_id** IS NOT NULL THEN suspensions.taxlabel ELSE suspensions_all.taxlabel END) AS suspension_taxlabel,
tak jak w innych polach tego zapytania co jest moim przeoczeniem i to poprawiłem. W takim przypadku COALECSE() jest nieuzasadnione. To co było wcześniej działało, ale mogło niespodziewanie się zachować w pewnych nietypowych sytuacjach.
Wprowadziłem COALECSE() dla innego pola w tym zapytaniu, bo się dało.
doc/lms.pgsql
Outdated
@@ -3818,6 +3854,311 @@ CREATE TRIGGER cash_customerbalances_update_trigger AFTER INSERT OR UPDATE OR DE | |||
CREATE TRIGGER cash_customerbalances_truncate_trigger AFTER TRUNCATE ON cash | |||
EXECUTE PROCEDURE customerbalances_update(); | |||
|
|||
CREATE VIEW vassignmentssuspensionsgroupcounts AS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kiedyś mielibyśmy customersview
i parę innych tego typu nazw - przeszliśmy na customerview
i podobne.
Nie warto wracać do tego, co już było i niczego przydatnego nie dawało. W angielskim obydwie formy są poprawne, ale krótsza jest lepsza.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poprawione
AND suspended = 1 AND commited = 1) | ||
))'; | ||
$state_conditions[] = 'EXISTS (SELECT 1 | ||
FROM assignments a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brak wcięć następnego wiersza PHP. W innych miejsach również (wielu).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tutaj też nie ma wcięć https://github.com/chilek/lms/pull/2474/files/f879ddf7cb1dbd4a0cc6c2d7fdc1599f9cc237da#diff-dbd33f9c8352f6b8ea14a53cf973174675da8d9fb5d2b1f30bdfd882620f6f04L977-L984
więc zastosowałem takie jak wszędzie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poprawiłem kilka kolejnych miejsc z wcięciami.
lib/definitions.php
Outdated
@@ -1666,6 +1666,25 @@ function ($link_technology) { | |||
5 => 'permanent residence card', | |||
); | |||
|
|||
// assignment suspensions | |||
const SUSPENSION_CHARGE_METHOD_NONE = 1, | |||
SUSPENSION_CHARGE_METHOD_ONCE = 2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dlaczego brak wcięć w kolejnych wierszach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poprawione
…OALESCE in some places
No description provided.