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

email filter by nights #67

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

email filter by nights #67

wants to merge 2 commits into from

Conversation

0x17de
Copy link
Collaborator

@0x17de 0x17de commented Oct 19, 2017

See issue #1

Copy link
Owner

@TimRepke TimRepke left a comment

Choose a reason for hiding this comment

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

  • Funktionen aus Environmend und anderen core-Klassen nutzen
  • Meedoo Nutzung prüfen - inkonsistent! lieber erst alten stil beibehalten und in PR updated the medoo db library #66 alles im gesamten projekt anpassen
  • Logik bzgl Kombination von Tagen prüfen (ggf noch and/or/xor toggle für meh overkill)

@@ -37,10 +37,13 @@ public function getText() {
foreach ($this->environment->oconfig['essen'] as $key => $typ) {
$essen .= '<option value="' . $key . '">' . $typ . '</option>';
}
$maxtage = $this->fahrt->getLenTage();
$fahrt_bereich = $this->environment->database->select('fahrten', ['von', 'bis'], ['fahrt_id' => $this->fahrt->getID()]);
Copy link
Owner

Choose a reason for hiding this comment

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

Nicht so elegant...
Lieber die entsprechende Funktion aus dem Fahrt-Objekt nehmen (bzw neu schreiben)! Ist einheitlicher und wenn sich was ändert, dann nur an einer Stelle. Dafür ist die Klasse ja da.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sehe ich mir an

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nutzt fahrt->getPossibleDates() in der neuen version

@@ -138,7 +141,7 @@ private function transformContacts() {
}

private function buildQueryWhere() {
$where = ['fahrt_id' => $this->fahrt->getID(), 'OR' => ['on_waitlist' => 0,
$where = ['fahrt_id' => $this->fahrt->getID(), 'OR #waitlist' => ['on_waitlist' => 0,
Copy link
Owner

Choose a reason for hiding this comment

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

was ist das denn mit der raute? neue medoo Syntax?
Sieht so aus, als sollte da on_waitlist stehen (?)

Copy link
Collaborator Author

@0x17de 0x17de Oct 20, 2017

Choose a reason for hiding this comment

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

das mit kommentar muss so. es gibt ja nun auch ein OR weiter unten bei nights. zwei or MUSS man mit medoo so lösen leider. wegen unique map keys

Copy link
Owner

Choose a reason for hiding this comment

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

kannte ich noch nicht, dachte das wäre nur irgendwie die neue syntax oder so.

$nights = $_REQUEST['val_nights'];
$conditions = [];
foreach ($nights as $night)
$conditions['AND #'.$night] = ['anday[<=]' => $night, 'abday[>]' => $night];
Copy link
Owner

Choose a reason for hiding this comment

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

Definitionssache, aber ich finde, dass die Logik falsch ist.
Wenn ich wähle, dass ich an Personen mailen will die an Tag 1, 2, und 3 da sind, dann meine ich nicht 'und' an allen Tagen, sondern meine die Leute, die an min einem der gwählten Tage da sind (glaube ich). Trippy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah. gute frage - ja, hier ist angenommen: an mindestens einer der selektierten nächte da. nicht gesagt 'NUR' in den nächten da.

foreach ($nights as $night)
$conditions['AND #'.$night] = ['anday[<=]' => $night, 'abday[>]' => $night];
if (sizeof($conditions) > 0)
$where['OR #nights'] = $conditions;
Copy link
Owner

Choose a reason for hiding this comment

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

warum ist hier ein OR und bei allen anderen keys in $where nicht?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wegen 'in mindestens einer selektierten nacht anwesend' logik

Copy link
Owner

Choose a reason for hiding this comment

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

Ja, das stimmt schon, bin mir nur nicht sicher, ob man lustige Seiteneffekte bekommt (Klammern/Scopes).

Beispiel: [[bezahlt] UND [auto] ODER [nacht1] ODER [nacht1 UND nacht2]]
Wäre ja doof dann, weil die beiden anderen (bezahlt, auto) irrelevant werden.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

die oberste ebene ist via und verknüpft - darunter sind die einzelnen nächte via oder verknüpft. also hat keine aushebelnde wirkung auf die bisherige selektion

for ($cnt = $maxtage; $cnt >= 0; $cnt--)
$tage .= '<option value="' . $cnt . '">' . $cnt . '</option>';
for ($tag = $von; $tag < $bis; $tag += 24*60*60)
$tage .= '<option value="' . date('Y-m-d', $tag) . '">' . date('d.m.Y', $tag) . '</option>';
Copy link
Owner

Choose a reason for hiding this comment

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

warum zwei verschiedene datumsformate? ist das in der DB so? mir ist so, als wäre da irgendwo im environment eine funktion für datumsangaben. wäre wünschenswert die zu nutzen!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

datumsformate sind so in der db - das rechte ist 'menschenleserlich'

@TimRepke TimRepke assigned 0x17de and unassigned TimRepke Oct 20, 2017
@0x17de 0x17de assigned TimRepke and unassigned 0x17de Oct 20, 2017
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.

2 participants