Calculating start of semester correctly

[viktorcsimma]

Today, we've received a report that the evaluation form has closed on 31st January instead of 3rd February. I could reopen it only by setting send_status_statement_request in the event_triggers table from 7th May to 7th October.

My suspect is the isEvaluationAvailable function in SemesterEvaluationController. Particularly this point:

public static function isEvaluationAvailable(): bool
{
    $available = EventTrigger::find(EventTrigger::SEMESTER_EVALUATION_AVAILABLE)->date;
    $deadline = self::deadline();

    return now() <= $deadline && $available >= Semester::next()->getStartDate();
}

I'll check this out.

[kdmnk]

What’s the value of the event triggers? And what’s the deadline’s value now? What’s in the .env as deadline? The send_status_statement_request should be in may, but the deactivate status signal is still 3rd of Feb? I’m not available to help today, just set isEvaluationAvailable() to return true temporarily.

…

On 1 Feb 2024, at 11.51, Viktor Csimma ***@***.***> wrote: Today, we've received a report that the evaluation form has closed on 31st January instead of 3rd February. I could reopen it only by setting send_status_statement_request in the event_triggers table from 7th May to 7th October. My suspect is the isEvaluationAvailable function in SemesterEvaluationController. Particularly this point: public static function isEvaluationAvailable(): bool { $available = EventTrigger::find(EventTrigger::SEMESTER_EVALUATION_AVAILABLE)->date; $deadline = self::deadline(); return now() <= $deadline && $available >= Semester::next()->getStartDate(); } I'll check this out. — Reply to this email directly, view it on GitHub <#421>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANMZKMPZO46PUNZITQTPTGLYRNXSZAVCNFSM6AAAAABCUVKEQ2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGEYTEMJZGI4DGNA>. You are receiving this because you are subscribed to this thread.

[viktorcsimma]

.env has SEMESTER_EVALUATION_DEADLINE="2024-02-03 23:59:59". The deactivate status signal really is the 3rd of February.

I think I can check this out tomorrow. But you are right; I'll circumvent isEvaluationAvailable and restore the dates in the database. No, I'll leave it in the database – with the container system, we would have to commit that to the deployment branch.

[kdmnk]

Also please check what's the end of the autumn and start of spring semester now.

…

________________________________ From: Viktor Csimma ***@***.***> Sent: Thursday, February 1, 2024 11:51:24 AM To: EotvosCollegium/mars ***@***.***> Cc: Subscribed ***@***.***> Subject: [EotvosCollegium/mars] Evaluation form closes early if `send_status_statement_request` is in the same semester (Issue #421) Today, we've received a report that the evaluation form has closed on 31st January instead of 3rd February. I could reopen it only by setting send_status_statement_request in the event_triggers table from 7th May to 7th October. My suspect is the isEvaluationAvailable function in SemesterEvaluationController. Particularly this point: public static function isEvaluationAvailable(): bool { $available = EventTrigger::find(EventTrigger::SEMESTER_EVALUATION_AVAILABLE)->date; $deadline = self::deadline(); return now() <= $deadline && $available >= Semester::next()->getStartDate(); } I'll check this out. — Reply to this email directly, view it on GitHub<#421>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANMZKMPZO46PUNZITQTPTGLYRNXSZAVCNFSM6AAAAABCUVKEQ2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGEYTEMJZGI4DGNA>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[BertalanD]

There are multiple strange things here:

• In the database, the signal with id 1 is called send_status_statement_request, while the code refers to it as SEMESTER_EVALUATION_AVAILABLE:
  

  mars/app/Models/EventTrigger.php

  Lines 54 to 57 in 04b9359

  	public const SIGNALS = [
  	self::SEMESTER_EVALUATION_AVAILABLE,
  	self::DEACTIVATE_STATUS_SIGNAL,
  	];

  
  Is there some migration we forgot to run on the database?

• It looks like the start of spring semester is hard-coded to start in the first week of February (wtf?!). I believe the reason this bug occurred is that we now rolled over to Semester::next()->getStartDate(); being in September.
  

  mars/app/Models/Semester.php

  Lines 130 to 143 in 04b9359

  	/**
  	* Returns a semester's start date: the starting month's (based on constants) first week's last day.
  	*/
  	public function getStartDate(): Carbon
  	{
  	$year = $this->year;
  	if ($this->isSpring()) {
  	$year += 1;
  	}
  	$month = $this->isAutumn() ? self::START_OF_AUTUMN_SEMESTER : self::START_OF_SPRING_SEMESTER;
  	return Carbon::createFromDate($year, $month, 1)->endOfWeek();
  	}

[kdmnk]

The name in the database does not matter, you can just rename it. Yes, it’s the first week’s end in February, which is the 4th now. The deadline of the form should be the day before, which is 3rd - that seems to be correct. The current semester should still be the autumn one, can you confirm?

…

On 1 Feb 2024, at 12.06, Daniel Bertalan ***@***.***> wrote: There are multiple strange things here: In the database, the signal with id 1 is called send_status_statement_request, while the code refers to it as SEMESTER_EVALUATION_AVAILABLE: https://github.com/EotvosCollegium/mars/blob/04b93590e0ce4b25017dcc3ec70dc69825f6f0a6/app/Models/EventTrigger.php#L54-L57 Is there some migration we forgot to run on the database? It looks like the start of sprint semester is hard-coded to start in the first week of February (wtf?!). I believe the reason this bug occurred is that we now rolled over to Semester::next()->getStartDate(); being in September. https://github.com/EotvosCollegium/mars/blob/04b93590e0ce4b25017dcc3ec70dc69825f6f0a6/app/Models/Semester.php#L130-L143 — Reply to this email directly, view it on GitHub <#421 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANMZKMKMECQE77FSWOHMK2DYRNZJVAVCNFSM6AAAAABCUVKEQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRRGA3DSOJZGY>. You are receiving this because you commented.

[viktorcsimma]

It was the next one, 2023/2024/2.

But now it seems OK. The hotfix was that we kinda hard-coded the semester into the code:

public static function current(): Semester
{
    $today = Carbon::today()->format('Ymd');
    if (!Cache::get('semester.current.' . $today)) {
        $now = Carbon::now();
        /*
        if ($now->month >= self::START_OF_SPRING_SEMESTER && $now->month <= self::END_OF_SPRING_SEMESTER) {
            $part = "2";
            $year = $now->year - 1;
        } else {
            $part = "1";
            // This assumes that the semester ends in the new year.
            $year = $now->month <= self::END_OF_AUTUMN_SEMESTER ? $now->year - 1 : $now->year;
        }
        */
        // hard-coding the current semester as a hotfix
        $year = $now->year - 1;
        $part = ($now->month > 2 || $now->day > 3) ? "2" : "1";
        $current = Semester::getOrCreate($year, $part);

        Cache::put('semester.current.' . $today, $current, Carbon::tomorrow());
    }

    return Cache::get('semester.current.' . $today);
}

So that was the problem. After the database change, it offered the form for the next semester (23/24/2), but now it seems fine.

We should think about how we could reliably determine the end of semesters. My first idea is to simply write the current semester into .env. What do you think?

[kdmnk]

The exact dates of the semesters do not matter for us. I just wonder how the end of the form desynchronised from the end of the semester.
If I'm not wrong, the code you commented out hasn't changed in years. We also have tests for that.

If you want to have exact semester dates, I'd rather make an admin page for that, and we will still need some default logic (what we currently have).

[horcsinbalint]

The codebase was probably written with the assumption that the first semester ends not later than the 30th of January.

On 2024-02-04 at 13:00:00, the code made all students alumnis as it beleived the current semester was 2023/2024/2 and it required a status for the next semester (2024/2025/1). I believe that there should be a seperate page where an administrator can review such changes before they are made.

I believe that such destructive actions should not be done automatically but rather under human supervision, after the operator makes a backup of all the important information.