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

fix: מפה לפי קו - לא מוצגות נסיעות לאחר השעה 6 בערב #902

Closed
witty-code opened this issue Oct 31, 2024 · 8 comments · Fixed by #936
Closed
Assignees
Labels
bug Something isn't working frontend frontend developers issue good first issue Good for newcomers

Comments

@witty-code
Copy link

יש כרגע בעיה עם הממשק בעמוד של מפה לפי קו, הוא לא מציג נסיעות שמתחילות לאחר השעה 6 בערב.

לאחר בדיקה איתרתי את הבאג: הפונקציה useSingleLineData יוצרת זמן התחלה לתחילת היום וזמן סיום עבור הרגע האחרון של היום:

const { locations, isLoading: locationsAreLoading } = useVehicleLocations({
    from: +new Date(timestamp).setHours(0, 0, 0, 0),
    to: +new Date(timestamp).setHours(23, 59, 59, 999),
    lineRef,
    splitMinutes: 360,
    pause: !lineRef,
  })

הפונקציה useVehicleLocations מחלקת את טווח הזמן לחלקים שווים לפי הערך split באמצעות הפונקציה getMinutesInRange:

function getMinutesInRange(from: Dateable, to: Dateable, gap = 1) {
  const start = moment(from).startOf('minute')
  const end = moment(to).startOf('minute')

  // array of minutes to load
  const minutes = Array.from({ length: end.diff(start, 'minutes') / gap }, (_, i) => ({
    from: start.clone().add(i * gap, 'minutes'),
    to: start.clone().add((i + 1) * gap, 'minutes'),
  }))
  return minutes
}

הבעיה שנוצרת היא שהקוד:

length: end.diff(start, 'minutes') / gap

מחזיר במקרה הזה את הערך 3.9972222222222222, אשר מתורגם כנראה בתור ערך מספרי 3, נסה:

parseInt(end.diff(start, 'minutes') / gap)

זה גורם לכך שנשלחות לשרת בקשות של 3 טווחי זמן של 6 שעות, במקום 4 כפי הנדרש.

יש 2 פתרונות אפשריים, אבל מכיוון שלא חקרתי את כל הקוד אינני יכול להמליץ על אחד מהם, ולכן אני מניח כאן את הדברים במקום לבצע Fork פשוט:

א. לבצע שינוי בפונקציה useSingleLineData כך שערך הסיום יוגדר לשנייה הראשונה של היום הבא, לדוגמא:

to: +new Date(timestamp + 24 * 60 * 60 * 1000).setHours(0, 0, 0, 0);

ב. לשנות את הפונקציה getMinutesInRange ולהחיל round על תוצאות החלוקה ב-gap (נראה פחות מומלץ, יש לבדוק את ההשלכות על כל מקרי השימוש במרבי הממשק):

function getMinutesInRange(from, to, gap = 1) {
  const start = moment(from).startOf('minute')
  const end = moment(to).startOf('minute')

  // array of minutes to load
  const minutes = Array.from({ length: Math.round(end.diff(start, 'minutes') / gap) }, (_, i) => ({
    from: start.clone().add(i * gap, 'minutes'),
    to: start.clone().add((i + 1) * gap, 'minutes'),
  }))
  return minutes
}
@witty-code witty-code changed the title fix: לא מוצגות נסיעות לאחר השעה 6 בערב fix: מפה לפי קו - לא מוצגות נסיעות לאחר השעה 6 בערב Nov 2, 2024
@NoamGaash NoamGaash added frontend frontend developers issue bug Something isn't working good first issue Good for newcomers labels Nov 15, 2024
@NoamGaash
Copy link
Member

מעולה! great catch 👏
ממליץ לקחת את הissue הזה לכל מי שרוצה לצלול לתוך החלקים javascript שלנו - אעשה לו pinned.

@NoamGaash NoamGaash pinned this issue Nov 15, 2024
@TomRytt
Copy link
Collaborator

TomRytt commented Nov 19, 2024

היי נועם, אני אשמח לטפל בזה :) מתחיל עכשיו

@TomRytt TomRytt self-assigned this Nov 19, 2024
@TomRytt
Copy link
Collaborator

TomRytt commented Nov 19, 2024

היי נועם,
פתרתי את הבאג בעזרת הטכניקה שהועלתה כאן עם שינוי קל:

const today = new Date(timestamp)
  const tomorrow = new Date(today)
  tomorrow.setDate(tomorrow.getDate() + 1)
  const { locations, isLoading: locationsAreLoading } = useVehicleLocations({
    from: +today.setHours(0, 0, 0, 0),
    to: +new Date(tomorrow).setHours(0, 0, 0, 0),
    lineRef,
    splitMinutes: 60,
    pause: !lineRef,
  })

אחרי שסידרתי את זה שמתי לב שיש באג שהשעה 11:00 PM ו-11:30 PM מופיעות ראשונות וזה חירפן אותי. שברתי על זה את הראש ועל הדרך עשיתי אופטימיזציות לפילטור והסידור של השעות כ-options בתוך ה-select:

פונקציה יוטילית להמרה לבסיס 24:

function convertTo24HourAndToNumber(time: string): number {
    const match = time.match(/(\d+):(\d+):(\d+)\s(AM|PM)/)
    if (!match) return 0

    const [, hour, minute, , modifier] = match
    let newHour = parseInt(hour, 10)
    if (modifier === 'AM' && newHour === 12) newHour = 0
    if (modifier === 'PM' && newHour !== 12) newHour += 12

    return newHour * 60 + parseInt(minute, 10)
  }

הקוד החדש שמייצר את ה-options:

const options = useMemo(() => {
    const filteredPositions = positions.filter(
      (position) =>
        !!position.recorded_at_time && position.recorded_at_time > +today.setHours(0, 0, 0, 0),
    )

    const uniqueTimes = Array.from(
      new Set(
        filteredPositions
          .map((position) => position.point?.siri_ride__scheduled_start_time)
          .filter((time): time is string => !!time)
          .map((time) => time.trim()),
      ),
    )
      .map((time) => new Date(time).toLocaleTimeString()) // Convert to 24-hour time string
      .map((time) => ({
        value: time,
        label: time,
      }))

    const sortedOptions = uniqueTimes.sort(
      (a, b) => convertTo24HourAndToNumber(a.value) - convertTo24HourAndToNumber(b.value),
    )

    return sortedOptions
  }, [positions])

יש בעיה שה-loader עובד בלי סוף. אני לא בטוח אם זה באג ישן או חדש... זה שווה בדיקה. אם תגיד לי שמה שפה לדעתך נותן מענה ונראה סבבה אני אפתח pull request מסודר (יכול גם קודם לפתוח, מה שתעדיף)

@witty-code
Copy link
Author

הסיבה שבגינה השעות 11:00 PM ו-11:30 PM מופיעות ראשונות היא משום שהשאילתה מחזירה תוצאות של שידורי GPS החל מהשעה 00:00 בתחילת היום שנבחר ועד השעה 23:59 (או 00:00 לאחר התיקון המוצע) בסוף היום.
היות והשאילתה מתבצעת לפי שעת שידור המיקום ולא לפי שעת היציאה המתוכננת, נכללות בתוצאת השאילתה גם נסיעות מיום קודם שנמשכו לאחר חצות (00:00), נסיעות אלו מופיעות בתוצאות החיפוש באופן חלקי - מופיעים רק רישומי ה-GPS שנקלטו לאחר השעה 00:00. במקביל - נסיעות שבוצעו ביום של תאריך החיפוש אך נמשכו לאחר השעה 00:00 מופיעות במקוטע - מופיעים רק רישומי ה-GPS של נסיעות אלו אשר נקלטו עד השעה 00:00. הסיבה כאמור - שאילתה למסד הנתונים מתבצעת לפי שעת השידור של המיקום ולא לפי שעת היציאה.

הבלגן מתחיל כאשר הפונקציה ממיינת את מיקומי ה-GPS לפי שעת היציאה של הנסיעה, ולכן נסיעות של יום קודם מופיעות יחד עם נסיעות של היום הנוכחי, במידה והן התחילו בשתי הימים העוקבים באותה השעה.
לדוגמא: קו 92 יצא מתחנת המוצא בשעה 23:30 בתאריכים העוקבים: 11/11/2024 ו-12/11/2024 , בחיפוש אחר נסיעות של קו זה עבור תאריך 12/11/2024 ה-dropdown יציג פעמיים את שעת היציאה 23:30, כאשר בחירה בכל אחד מהם תציג נסיעה בודדת ורציפה כביכול:

  1. היא תכלול את רישומי ה-GPS של הנסיעה שהחלה ב-11/11/2024 בשעה 23:30, עבור נסיעה זו יופיעו מיקומים שנקלטו לאחר השעה 00:00. רכב 9265601.
  2. היא תכלול את רישומי ה-GPS של הנסיעה שהחלה ב-12/11/2024 בשעה 23:30, עבור נסיעה זו יופיעו רק מיקומים שנקלטו עד השעה 00:00 בסוף היום. רכב 9234801.

אני סבור ששאילתת החיפוש צריכה להתחיל ולסיים בשעה 04:00 לפנות בוקר, אשר היא השעה הרלוונטית להתחלפות 'יום תחבורה' לפי משרד התחבורה.

@TomRytt
Copy link
Collaborator

TomRytt commented Nov 20, 2024 via email

@NoamGaash
Copy link
Member

NoamGaash commented Nov 20, 2024

🤯
מדהים, תודה רבה על החקר הזה. ההבחנה בין זמן יציאה מתוכננת לזמן שידור המיקום היא חשובה ובאמת התפספסה לאורך הדרך. לא ידעתי שיש שעת החלפת יום רשמית של משרד התחבורה - שמעתי שהם מוסיפים שעה 25 ליממה במקרים מסויימים. יש לך מקור לזה? זה ממש מגניב אם זה משהו שאפשר להסתמך עליו.
תודה רבה לשניכם 🥇

@witty-code
Copy link
Author

המקור שלי הוא מהתיעוד הרשמי של משרד התחבורה בקישור הזה בסעיף 2.8.2, וכן בסעיף 9.2 :

Question: There are cases in which the field arrival_time at ‘stop_times’ file are above 24 hours, for example: 25:31:00

Answer: This is not an error. The logic of the field arrival_time is dictated by the GTFS protocol and is carried out as follows:
A trip that begins before midnight and continues after midnight - all of its station's timing will appear on the same calendar day it started, and the hours are 24, 25, 26, etc ...
A trip that begins after midnight, passes to the next calendar day. And the hours will be 1 2, 3 ... etc.
In terms of the key, after understanding the above logic, you can define in the app itself that every trip that appears between 00:00 and 04:00 will be displayed as the night line.
It should be noted that regular public transportation starts at 04:00 in the morning, except in rare cases.

אפשר לחלוק על הפרשנות הזו, היות שבפועל נסיעות שמתחילות לאחר חצות מתוארכות ליום הקלנדרי שבו החלו ולא ליום הקודם.

בנוסף, בצו פיקוח על מחירי מצרכים ושירותים (דמי נסיעה ברכבת וברכבל), התשפ״ב–2022, סעיף 1(א) :

”יום“ – לעניין חיוב תשלום באמצעי תשלום מתקדם – פרק הזמן שמשעה 04:00:00 עד שעה 03:59:59 ביום העוקב;

@TomRytt
Copy link
Collaborator

TomRytt commented Nov 20, 2024

בהמשך לשיחה שלנו, החלפתי את recorded_at_time בפונקציה עם position.point?.siri_ride__scheduled_start_time וגם שיפרתי את הפילטר ויצאתי מהפונקציה במידה והפילטור החזיר מערך ריק (לפני שבוחרים קו)

מהבדיקה שערכתי זה גם עבד מצוין וגם הרבה יותר מהיר מבעבר (הרבה פחות קווים עוברים את הפילטור הראשוני)

זה השינוי המרכזי:

    const filteredPositions = positions.filter((position) => {
      const startTime = position.point?.siri_ride__scheduled_start_time
      return !!startTime && +new Date(startTime) > +today.setHours(0, 0, 0, 0)
    })

    if (filteredPositions.length === 0) return []

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working frontend frontend developers issue good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants