Correction for undefined variable $record in modules\system\behaviors\SettingsModel.php line 140

[Quendi6]

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.

A correct rewrite could be :

    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.

[LukeTowers]

@Quendi6 can you submit this as a PR instead? It's much easier to collaborate on proposed code changes in a PR rather than an issue.

[Quendi6]

This was a "Pre-Pull Request Discussion". I assume it was not necessary. I'm a bit new to this, hope I'm doing it well. Here the pull request: https://github.com/wintercms/winter/pull/1134

[LukeTowers]

Closed by #1134