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

make Orderprocess more stable #194

Merged
merged 14 commits into from
Sep 12, 2023
Merged

Conversation

mariolorenz
Copy link
Collaborator

... instead of running direct Execute. So Order is not created ...

... instead of running direct Execute. So Order is not created ...
@Sieg Sieg added the internal label Sep 6, 2023
@mariolorenz mariolorenz changed the title WIP: use getExecuteFnc make Orderprocess more stable Sep 8, 2023
@@ -67,10 +67,6 @@ public function render()
*/
public function execute()
{
if (!$this->isSepaConfirmed()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warum entfernst du diesen code, im ticket kann ich auf den ersten blick nicht über sepa finden

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alle Unzer-Bestellungen gehen über die neue "executeoscunzer". Da ist auch der SEPA-Check mit drin.

Einzig Apple-Pay nutzt aus meiner Sicht noch die "execute". Das kann ich leider nur nicht richtig testen ...

@@ -87,45 +83,47 @@ public function execute()
$response->setData([
'redirectUrl' => $unzer->prepareRedirectUrl('thankyou')
])->sendJson();
} elseif ($this->isSepaPayment()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

siehe oben:
warum entfernst du diesen code, im ticket kann ich auf den ersten blick nicht über sepa finden

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

s.o.

if ('thankyou' === $nextStep) {
$oDB->commitTransaction();
} else {
$oDB->rollbackTransaction();
Copy link
Contributor

Choose a reason for hiding this comment

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

evtl. ist hier weiterhin ein Registry::getUtilsView()->addErrorToDisplay($oEx); sinnvoll?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Das ist eine gute Idee. Das ergänze ich ...

/**
* execute Unzer defined via getExecuteFnc
*/
public function executeoscunzer(): ?string
Copy link
Contributor

Choose a reason for hiding this comment

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

fände camel case lesbarer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm. Das ist eine Methode die als fnc gestartet wird. Im Core werden diese Methoden alle in "klein" geschrieben. Möglicherweise ist es egal. Ich habe mich hier an den Core gehalten. Könnte ich aber ausprobieren ...

@@ -117,14 +118,6 @@ public function executeUnzerPayment(PaymentModel $paymentModel): bool
$basket
);

/** @var string $sess_challenge */
Copy link
Contributor

Choose a reason for hiding this comment

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

ist dieser code wirklich so nicht mehr notwendig?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Das schreiben der "writeTransactionToDB" passiert an anderer Stelle. Später erst, wenn die Order auch vorhanden ist. Hier sorgt sie sogar für richtig Murks, denn da die Order ja noch nicht existiert legt die Methode einfach eine 0-Order an...

Und noch etwas generell gibt es keine 0-Orders mehr. Darum braucht es auch die Aufräumfunktion removeTemporaryOrder an der Stelle nicht mehr ...

@sonarcloud
Copy link

sonarcloud bot commented Sep 12, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

5.6% 5.6% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@mariolorenz mariolorenz merged commit 8804d88 into b-6.3.x Sep 12, 2023
12 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants