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

Mark $imageId as mixed to prevent typing errors. Closes #57. #58

Merged
merged 1 commit into from
May 9, 2022

Conversation

davidwebca
Copy link
Contributor

A typing error was triggered because some image queries can be arrays and others are straight up image ids (strings), so changing the type to mixed solves this issue.

@davidwebca davidwebca requested a review from khalwat as a code owner May 9, 2022 17:41
@khalwat
Copy link
Contributor

khalwat commented May 9, 2022

Definitely appreciate the PR -- I think it probably should be changed to ?int though... and Craft should take care of properly typecasting the strings from the db into ints via helpers/Typecast

We also should do this for the $videoId

I'll merge in the changes, then revise as per the above on my end.

@khalwat khalwat merged commit 25859bb into nystudio107:develop-v4 May 9, 2022
@davidwebca
Copy link
Contributor Author

No worries. You're right, ?int should be okay because of the base model type casting - I just noticed this. I'm making another pass right now. There's another typing error when trying to save because the base model doesn't accept an instance anymore apparently? 🤔

@davidwebca
Copy link
Contributor Author

Mmm nevermind. I just noticed another typing exception when selecting an image. I had to re-assign the volumes to the field (I expected that because of the latest changes they made to volumes), but now I also get the array / int exception just after selecting an image or video, in the ajax function.

{
    "error": "Cannot assign array to property nystudio107\\recipe\\models\\Recipe::$imageId of type ?int",
    "exception": "TypeError",
    "file": "/vendor/yiisoft/yii2/BaseYii.php",
    "line": 558,
    "trace": [
    {
        "file": "/vendor/yiisoft/yii2/base/BaseObject.php",
        "line": 107,
        "function": "configure",
        "class": "yii\\BaseYii",
        "type": "::"
    },
    {
        "file": "/vendor/craftcms/cms/src/base/Model.php",
        "line": 78,
        "function": "__construct",
        "class": "yii\\base\\BaseObject",
        "type": "->"
    },
    {
        "file": "/Users/dee/Projets/Craft/craft-recipe/src/fields/Recipe.php",
        "line": 94,
        "function": "__construct",
        "class": "craft\\base\\Model",
        "type": "->"
    },
    {
        "file": "/vendor/craftcms/cms/src/base/Element.php",
        "line": 4661,
        "function": "normalizeValue",
        "class": "nystudio107\\recipe\\fields\\Recipe",
        "type": "->"
    },
    {
        "file": "/vendor/craftcms/cms/src/base/Element.php",
        "line": 3849,
        "function": "normalizeFieldValue",
        "class": "craft\\base\\Element",
        "type": "->"
    },
    {
        "file": "/vendor/craftcms/cms/src/controllers/ElementsController.php",
        "line": 1625,
        "function": "setFieldValuesFromRequest",
        "class": "craft\\base\\Element",
        "type": "->"
    },
    {
        "file": "/vendor/craftcms/cms/src/controllers/ElementsController.php",
        "line": 1110,
        "function": "_applyParamsToElement",
        "class": "craft\\controllers\\ElementsController",
        "type": "->"
    },
    {
        "function": "craft\\controllers\\{closure}",
        "class": "craft\\controllers\\ElementsController",
        "type": "->"
    },
    {
        "file": "/vendor/yiisoft/yii2/db/Connection.php",
        "line": 817,
        "function": "call_user_func"
    },
    {
        "file": "/vendor/craftcms/cms/src/controllers/ElementsController.php",
        "line": 1197,
        "function": "transaction",
        "class": "yii\\db\\Connection",
        "type": "->"
    },
    {
        "function": "actionSaveDraft",
        "class": "craft\\controllers\\ElementsController",
        "type": "->"
    },
    {
        "file": "/vendor/yiisoft/yii2/base/InlineAction.php",
        "line": 57,
        "function": "call_user_func_array"
    },
    {
        "file": "/vendor/yiisoft/yii2/base/Controller.php",
        "line": 178,
        "function": "runWithParams",
        "class": "yii\\base\\InlineAction",
        "type": "->"
    },
    {
        "file": "/vendor/yiisoft/yii2/base/Module.php",
        "line": 552,
        "function": "runAction",
        "class": "yii\\base\\Controller",
        "type": "->"
    },
    {
        "file": "/vendor/craftcms/cms/src/web/Application.php",
        "line": 301,
        "function": "runAction",
        "class": "yii\\base\\Module",
        "type": "->"
    },
    {
        "file": "/vendor/craftcms/cms/src/web/Application.php",
        "line": 625,
        "function": "runAction",
        "class": "craft\\web\\Application",
        "type": "->"
    },
    {
        "file": "/vendor/craftcms/cms/src/web/Application.php",
        "line": 280,
        "function": "_processActionRequest",
        "class": "craft\\web\\Application",
        "type": "->"
    },
    {
        "file": "/vendor/yiisoft/yii2/base/Application.php",
        "line": 384,
        "function": "handleRequest",
        "class": "craft\\web\\Application",
        "type": "->"
    },
    {
        "file": "/web/index.php",
        "line": 22,
        "function": "run",
        "class": "yii\\base\\Application",
        "type": "->"
    }]
}

@khalwat
Copy link
Contributor

khalwat commented May 9, 2022

Ah yeah okay. Let me do some end to end testing

@davidwebca
Copy link
Contributor Author

No worries. You can look at another thing I had to remove to be able to select an asset: "enabledForSite" seems to have been removed on AssetQuery. develop-v4...davidwebca:develop-v4

@khalwat
Copy link
Contributor

khalwat commented May 9, 2022

@khalwat
Copy link
Contributor

khalwat commented May 9, 2022

@davidwebca haha yeah I independently found that one too in my testing (enabledForSite)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants