Skip to content
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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

rafalpietraszewicz
Copy link
Collaborator

No description provided.

Copy link
Owner

@chilek chilek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Ciekawe, jaki wpływ na wydajność będzie miało użycie tak mocno rozbudowanych, nowo dodawanych widoków.
  2. 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?
  3. 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.

@@ -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
Copy link
Owner

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poprawiłem.

FROM assignments a
JOIN customers c ON a.customerid = c.id
JOIN customers c ON (a.customerid = c.id)
Copy link
Owner

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poprawiłem.

" . ($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
Copy link
Owner

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.

Copy link
Collaborator Author

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,
Copy link
Owner

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()?

Copy link
Collaborator Author

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
Copy link
Owner

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.

Copy link
Collaborator Author

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
Copy link
Owner

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).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

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.

@@ -1666,6 +1666,25 @@ function ($link_technology) {
5 => 'permanent residence card',
);

// assignment suspensions
const SUSPENSION_CHARGE_METHOD_NONE = 1,
SUSPENSION_CHARGE_METHOD_ONCE = 2,
Copy link
Owner

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poprawione

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants