winter icon indicating copy to clipboard operation
winter copied to clipboard

Fix undefined variable $record when exception is not SQLSTATE[42S02] …

Open Quendi6 opened this issue 1 year ago • 2 comments

…(wintercms#1132)

Package targeted

Winter CMS

Description

If the query fails not because of a base table or view not found, the getSettingsRecord() function tries to access a $record variable that is not defined.

Here is an excerpt commenting on the changes:

    public function getSettingsRecord()
    {
        $query = $this->model->where('item', $this->recordCode);

        if ($this->cacheTtl > 0) {
            $query = $query->remember($this->cacheTtl, $this->getCacheKey());
        }

        $record = null; // PR: "$record = null;" defined outside of Try/Catch block.

        try {
            $record = $query->first();
        } catch (QueryException $ex) {
            // SQLSTATE[42S02]: Base table or view not found - migrations haven't run yet
            if ($ex->getCode() === '42S02') {
                // PR: "$record = null;" removed from this "if" case ("$record" already set to "null" if exception caught)
                traceLog($ex);
            }
        }

        return $record ?: null; // PR: We still need a condition to return "null" in case of empty collection.
    }

// PR: comments are here to explain my modifications.

Maybe I'm wrong, but I think it will be better code writing

Will this change be backwards-compatible?

This seems fully backwards compatible since the return will have the same values, the only difference being that it will become impossible to encounter the "Undefined variable $record" error.

Quendi6 avatar May 22 '24 08:05 Quendi6

@Quendi6 what was the exception that you ran into that triggered this error? I specifically wrote it like that originally so that anyone getting a different exception would have to report it to me 😂

LukeTowers avatar May 22 '24 20:05 LukeTowers

@LukeTowers 😂In truth: none. But the way it was written triggered something in me. All the code is clean except for this, which drives me crazy.🤷‍♂️ May be some OCDs here...

Quendi6 avatar May 23 '24 08:05 Quendi6