wintercms / winter

Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework.
https://wintercms.com
MIT License
1.31k stars 188 forks source link

Code tidying #1134

Closed Quendi6 closed 1 month ago

Quendi6 commented 1 month ago

…(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.

LukeTowers commented 1 month ago

@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 πŸ˜‚

Quendi6 commented 1 month ago

@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...