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

JSON REST "locations" route does not work when WP_DEBUG=1 #1466

Closed
nelarsen opened this issue Dec 15, 2023 · 17 comments · Fixed by #1469 or #1547
Closed

JSON REST "locations" route does not work when WP_DEBUG=1 #1466

nelarsen opened this issue Dec 15, 2023 · 17 comments · Fixed by #1469 or #1547
Assignees
Labels
bug Something isn't working
Milestone

Comments

@nelarsen
Copy link
Contributor

CB 2.8.6 installiert von WP Plugins Directory.

Wenn WP_DEBUG gesetzt ist, gibt /wp-json/commonsbooking/v1/locations ein leeres/ungültiges Ergebnis zurück und ein Stack Trace wird in der WP-Logdatei ausgegeben. Offenbar liegt es daran, dass die "locations"-Route validateData() aufruft wenn und nur wenn WP_DEBUG=1. validateData() ist jedoch nicht erfolgreich, weil ein schemaValidation() ausgefüht wird. schemaUrl ist definiert als

protected $schemaUrl = COMMONSBOOKING_PLUGIN_DIR . "node_modules/commons-api/commons-api.locations.schema.json";

und diese Datei ist nicht vorhanden.

Wenn WP_DEBUG nicht gesetzt ist, funktioniert die locations-Route. Bei 'availability' besteht das o.g. Problem nicht, weil schemaValidation() nicht aufgerufen wird.

@nelarsen nelarsen added the bug Something isn't working label Dec 15, 2023
@hansmorb
Copy link
Contributor

Dopplung mit #1408.
Warum wollt ihr denn CB2 mit WP_DEBUG aktiviert nutzen?

@nelarsen
Copy link
Contributor Author

Oh, sorry wegen der Doppelung. Wir sind noch in der Testphase und testen projektspezifische Plugins. Dafür ist WP_DEBUG hilfreich oder notwendig. Der beschriebene Bug verhindert das, weil ein Plugin von ebendieser locations-Route abhängig ist.. Später wollen wir WP_DEBUG nicht aktiviert lassen.

@chriwen
Copy link
Member

chriwen commented Dec 15, 2023

ich nehme mal an an, dass das commons-api Verzeichnis in den node-modules im Zuge der aktuelleren build-Prozess-Anpassungen mal irgendwann rausgeflogen ist. Das müssten wir wieder implementieren.

@hansmorb
Copy link
Contributor

hansmorb commented Dec 15, 2023

Ah, ich habe gar nicht daran gedacht: @nelarsen Probier doch mal statt einer Plugin Zip mit git clone das Plugin im wp-content/plugins Folder zu installieren und dann auf den release/2.8.6 Branch zu wechseln. Wenn du jetzt npm run start ausführst, sollte das Problem behoben sein.

@hansmorb
Copy link
Contributor

ich nehme mal an an, dass das commons-api Verzeichnis in den node-modules im Zuge der aktuelleren build-Prozess-Anpassungen mal irgendwann rausgeflogen ist. Das müssten wir wieder implementieren.

Na ja, ich würde eigentlich den node_modules Ordner im besten Fall gar nicht mit dem Plugin mitliefern. Wenn dann würde ich eher den Check deaktivieren, wenn er die Schema Datei nicht findet oder wenn der node_modules Ordner nicht existiert

@hansmorb hansmorb linked a pull request Dec 18, 2023 that will close this issue
@hansmorb
Copy link
Contributor

CB 2.8.6 installiert von WP Plugins Directory.

Wenn WP_DEBUG gesetzt ist, gibt /wp-json/commonsbooking/v1/locations ein leeres/ungültiges Ergebnis zurück und ein Stack Trace wird in der WP-Logdatei ausgegeben. Offenbar liegt es daran, dass die "locations"-Route validateData() aufruft wenn und nur wenn WP_DEBUG=1. validateData() ist jedoch nicht erfolgreich, weil ein schemaValidation() ausgefüht wird. schemaUrl ist definiert als

protected $schemaUrl = COMMONSBOOKING_PLUGIN_DIR . "node_modules/commons-api/commons-api.locations.schema.json";

und diese Datei ist nicht vorhanden.

Wenn WP_DEBUG nicht gesetzt ist, funktioniert die locations-Route. Bei 'availability' besteht das o.g. Problem nicht, weil schemaValidation() nicht aufgerufen wird.

Magst du mal den Bugfix im verlinkten Branch testen? Du kannst mit den bin/build-zip.sh eine Plugin zip aus dem ausgecheckten Branch erstellen

@nelarsen
Copy link
Contributor Author

nelarsen commented Dec 18, 2023

Magst du mal den Bugfix im verlinkten Branch testen? Du kannst mit den bin/build-zip.sh eine Plugin zip aus dem ausgecheckten Branch erstellen

Ich habe ein Workaround für mich für jetzt, aber ich teste natürlich sehr gern deine Lösung. Leider schaffe ich es nicht (mehr) die Dependencies mit composer install --no-dev oder build/bin-zip.sh zu installieren (gerade Fehler wegen "Cypress cannot write to the cache directory due to file permissions"). Könntest du mir die ZIP-Datei schicken?

@hansmorb
Copy link
Contributor

Hui, das ist ja ungünstig! Ja, hier ist die zip:
commonsbooking.zip

@nelarsen
Copy link
Contributor Author

Ich kann bestätigen, dass der bugfix/issue-1408-Branch das Problem löst. Vielen Dank!

@chriwen
Copy link
Member

chriwen commented Dec 19, 2023

@hansmorb mir ist noch nicht ganz klar warum du die Abfrage nach dem Schema im produktivbetrieb abschaltest. Braucht es das nicht? Wir hatten das Schema damals absichtlich lokal hinterlegt da ein Zugriff auf ein externes repo nicht den WP Sicherheitsrichtlinien entsprach. Wie bekommen wir das wieder eingebunden? Oder kann das generell weg? Bin da nicht so firm aber hätte vermutet dass es schon einen Sinn macht dass wir das vorhalten?

@chriwen
Copy link
Member

chriwen commented Dec 19, 2023

Evtl. müssen wir das Schema dann woanders hinlege wenn es nicht ins node_modules soll.

@chriwen chriwen reopened this Dec 19, 2023
@hansmorb
Copy link
Contributor

hansmorb commented Dec 19, 2023

Hmm, das wird aber sowieso nur geprüft, wenn WP_DEBUG aktiviert ist. Kenne mich damit auch nicht so super aus. Aber WP_DEBUG ist ja im Produktivbetrieb eigentlich deaktiviert. Sorry für den vorschnellen merge!

@hansmorb hansmorb added the triage label Feb 8, 2024
@datengraben datengraben added this to the 2.9.1 milestone Feb 15, 2024
datengraben added a commit that referenced this issue Mar 5, 2024
This is a fix for #1466; it moves common-api schema files at grunt-build time
to the assets folder and changes the relevant paths.
@datengraben
Copy link
Contributor

datengraben commented Mar 5, 2024

@nelarsen @hansmorb @chriwen
Mit 72ead46 sind jetzt die Schema-Dateien an einem Ort wo auch eine nicht-DEV-Installation sie lesen können.

EDIT: Der Validator verhält sich auch wie erwartet. Ich hatte noch einen Fehler in der Verwendung der neuen OPIS-Validator API. #1545

@hansmorb
Copy link
Contributor

hansmorb commented Mar 12, 2024

Leider immer noch nicht behoben, auf der aktuellen Release Zip bekomme ich noch folgenden Fehler:


Warning: file_get_contents(/var/www/vhosts/XXXXXX/wp-content/plugins/commonsbooking/assets/schemas/commons-api/commons-api.locations.schema.json): Failed to open stream: No such file or directory in /var/www/vhosts/XXXXXX/wp-content/plugins/commonsbooking/src/API/BaseRoute.php on line 134

Ich denke mal das Problem liegt darin, dass in den assets gesucht wird, die Dateien aber in den includes liegen:

image

@hansmorb hansmorb reopened this Mar 12, 2024
datengraben added a commit that referenced this issue Mar 13, 2024
@hansmorb
Copy link
Contributor

hansmorb commented Mar 14, 2024

Danke dir, die Locations Route funktioniert jetzt wieder!
Leider kriege ich jetzt bei der Items Route einen Validation Fehler (auch nur bei aktiviertem WP_DEBUG):
image

Das ist die gesamte Antwort aus unserem Produktivsystem bei freie Lasten:
items.json

@hansmorb
Copy link
Contributor

Ich glaube das Problem ist, dass ownerID für die items Route ein required Parameter ist, dieser aber nur gesetzt ist, wenn auch wirklich item Admins definiert sind:

image

FÜr mich die sauberste Lösung wäre das Schema entsprechend anzupassen, das müsste dann aber im Repo für das Schema passieren. Für die Locations ist das auch kein required Parameter.

@hansmorb
Copy link
Contributor

Ich schließe das jetzt hier, da das eine separate Issue ist!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
4 participants