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

Feature/holiday timeframe multiselect #1296

Merged
merged 68 commits into from
Feb 8, 2024

Conversation

hansmorb
Copy link
Contributor

@hansmorb hansmorb commented Jun 20, 2023

EDIT 2024-02-04 von @datengraben - Aufteilung via Überschriften und
EDIT 2024-02-08 von @datengraben - Name des Meta-Feld


TLDR; Ein Versuch, existierende Changes schon älterer Branches für das "Multiselect bei Holidays"-Feature neu aufzusetzen und sie mit Unit-Tests zu versehen.

Was

Ermöglicht Nutzenden mehrere Artikel / Standorte einem Urlaub Zeitrahmen zuzuordnen, dabei haben die Nutzenden die Auswahl zwischen folgenden Multi-Select-Typen:

  • Manueller Auswahl
  • Kategorie Auswahl
  • Alle (Timeframes)

Multi-Select-Typ: Manuelle Auswahl

image

Multi-Select-Typ: Kategorieauswahl

image

Multi-Select-Typ: "Alle"

image

Wie

Ein neues Metafeld namens "location-id-list" bzw. "item-id-list" wird angelegt, das existiert parallel zu "item-id" und "location-id" und kommt nur bei Holiday Zeitrahmen zum Tragen.
Für die Kategorie und Alle Auswahl wird beim Speichern des Timeframe automatisch das location-id-list bzw. item-id-list array mit den aktuellen Werten aktualisiert. (Also was aktuell zu diesem Zeitpunkt Artikel sind die in Kategorie sind oder die IDs von allen Artikeln).

Da unsere SQL Queries postIds brauchen, müssen wir das so machen. Also muss auch dieses location-id-list und item-id-list Metafeld regelmäßig aktualisiert werden. Das passiert wenn:

  • Beliebiger Timeframe bearbeitet / gelöscht / erstellt
  • Kategorie aus unserer Taxonomie gelöscht oder verschoben
  • Artikel / Standort aktualisiert

Mehr brauchen wir nicht, wenn ein Artikel / Standort gelöscht wird ist das nicht schlimm. Wenn eine Kategorie zugewiesen wird, wird der Artikel / Standort aktualisiert.

Warum nicht alle Zeitrahmen

Es gibt noch zu viele Funktionen, die bei Buchbaren Zeitrahmen immer nur GENAU einen Standort / genau ein Item erwarten. Die wurden mit TODO #507 markiert. Diese Stellen müssten geändert werden um eine Zuweisung auf Buchbare Zeitrahmen zu ermöglichen.

Darüber hinaus ergibt die Option "Alle Standorte" bei buchbaren Zeitrahmen keinen Sinn.

Refactored

Ich habe die Zugriffe auf die itemID & locationID von magic gettern zu einer eigenen Methode gemacht. Das hat den Hintergrund, dass wir bei mehreren Items für einen Timeframe nicht einfach getItems()->ID machen können, da getItems() immer eine Array zurückgibt. Aktuell funktioniert zwar noch getItem()->ID, aber wenn wir das später mal umbauen wollen ist es einfacher wenn das ein Methodenaufruf ist. Darüber hinaus gibt es Methoden wie getTags(), die den Metakey item-id bzw. location-id direkt ziehen, da wollte ich ermöglichen, dass das auch mit mehreren items/locations geht ohne immer die items / location posts aus der Datenbank ziehen zu müssen.

Diskutiert

Sollen beim Timeframe Export mehrere Items so dargestellt werden (obere Zeile)
image

oder soll der Timeframe mehrere Zeilen haben, jeweils immer mit anderen Werten?

EDIT: Plenum 29.10. :

  • Darstellung wie als wären es mehrere Zeitrahmen (Loop über jede mögliche Standort / Artikelkombo) TODO

Mögliche TO-Dos:

  • Anzeige in der Zeitrahmenübersicht reparieren

@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Attention: 62 lines in your changes are missing coverage. Please review.

Comparison is base (5939e90) 41.14% compared to head (ccd5efb) 43.14%.

Files Patch % Lines
src/Wordpress/CustomPostType/Timeframe.php 61.71% 49 Missing ⚠️
src/View/TimeframeExport.php 89.47% 6 Missing ⚠️
src/Model/Timeframe.php 95.91% 2 Missing ⚠️
src/Wordpress/CustomPostType/Item.php 71.42% 2 Missing ⚠️
src/View/Calendar.php 0.00% 1 Missing ⚠️
src/Wordpress/CustomPostType/Location.php 75.00% 1 Missing ⚠️
src/Wordpress/CustomPostType/Map.php 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1296      +/-   ##
============================================
+ Coverage     41.14%   43.14%   +1.99%     
- Complexity     2334     2384      +50     
============================================
  Files            91       91              
  Lines          9235     9462     +227     
============================================
+ Hits           3800     4082     +282     
+ Misses         5435     5380      -55     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hansmorb hansmorb linked an issue Jun 24, 2023 that may be closed by this pull request
12 tasks
@hansmorb hansmorb marked this pull request as ready for review July 9, 2023 14:00
# Conflicts:
#	src/Model/Timeframe.php
#	src/Wordpress/CustomPostType/CustomPostType.php
#	tests/Wordpress/CustomPostTypeTest.php
Copy link
Contributor

@datengraben datengraben left a comment

Choose a reason for hiding this comment

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

Hallo @hansmorb, ich habe nur Kleinigkeiten anzumerken.

Zusätzliche Anmerkung: Wieso nennen wir die postmeta-Strings der neuen Felder nicht location-id-list anstatt location-ids, dadurch sind sie etwas einfacher (durch mehr als einen Buchstaben) unterscheidbar.

src/Model/Timeframe.php Outdated Show resolved Hide resolved
src/Model/Timeframe.php Outdated Show resolved Hide resolved
src/Repository/Timeframe.php Outdated Show resolved Hide resolved
src/View/TimeframeExport.php Show resolved Hide resolved
src/View/TimeframeExport.php Show resolved Hide resolved
assets/admin/js/admin.js Show resolved Hide resolved
assets/admin/js/src/timeframe.js Outdated Show resolved Hide resolved
src/Helper/Wordpress.php Show resolved Hide resolved
src/Model/Timeframe.php Outdated Show resolved Hide resolved
src/Wordpress/CustomPostType/Timeframe.php Show resolved Hide resolved
@hansmorb
Copy link
Contributor Author

hansmorb commented Feb 7, 2024

Hallo @hansmorb, ich habe nur Kleinigkeiten anzumerken.

Zusätzliche Anmerkung: Wieso nennen wir die postmeta-Strings der neuen Felder nicht location-id-list anstatt location-ids, dadurch sind sie etwas einfacher (durch mehr als einen Buchstaben) unterscheidbar.

Das klingt gut, habe ich gleich mal umgesetzt

hansmorb and others added 3 commits February 7, 2024 21:24
# Conflicts:
#	tests/php/Wordpress/CustomPostTypeTest.php
Documents `@since` version in the private/public api and format the indentation correctly.
@hansmorb hansmorb merged commit 8d98c00 into master Feb 8, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request php Pull requests that update Php code
Projects
Development

Successfully merging this pull request may close these issues.

Holiday feature fertigstellen
3 participants