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

Tracked queries are not triggered when updating fields that are not in the query conditions #527

Closed
JodebaDigitalPulse opened this issue Jul 4, 2023 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@JodebaDigitalPulse
Copy link

1. Steps to reproduce the issue.

<h2>{{ craft.entries.section('jobs').count() }}</h2>
  • enable or disable an entry, so the count of enabled should change
  • after refresh job, count output will not be refreshed
  • the function helpers\RefreshCacheHelper::getElementTypeQueryRecords() will add a join with elementQueryFields and elementQueryAttributes as soon ($isChangedByAttributes || $isChangedByFields) is true
  • So change of status from enabled -> to disabled will have a positive result in this test
  • But elementQueryFields and elementQueryAttributes are only filled with attributes or fields used in the query to filter and order by.
  • So in order to limit the getElementTypeQueryRecords correctly, there should be a check for every field and attribute used in the templates (and always 'enabled'). Now it is only checked against filters and order by.

The plugin and Craft version numbers.

Craft 4.4.15 with Blitz 4.4.6

@JodebaDigitalPulse JodebaDigitalPulse added the bug Something isn't working label Jul 4, 2023
@bencroker
Copy link
Collaborator

Thanks for the explanation, but changing the entry status should trigger a cache refresh since both $isChangedByAttributes and $isChangedByFields will be false. Have you tested this and double-checked the steps followed?

@JodebaDigitalPulse
Copy link
Author

Indeed, $isChangedByFields is false, but $isChangedByAttributes is still true (but I think only in the case of having a preparse field) and the attributes 'enabled' and 'dateUpdated' are never in the table blitz_elementqueryattributes

A dump at the end of RefreshCacheHelper::getElementTypeQueryRecords

array:5 [
  "isChangedByAttributes" => true
  "isChangedByFields" => false
  "changedAttributes" => array:1 [
    0 => "enabled"
  ]
  "changedFields" => array:1 [
    0 => 171
  ]
  "sql" => """
    SELECT `blitz_elementqueries`.*\n
    FROM `blitz_elementqueries`\n
    INNER JOIN `blitz_elementquerycaches` ON `blitz_elementqueries`.`id` = `blitz_elementquerycaches`.`queryId`\n
    LEFT JOIN `blitz_elementquerysources` ON `blitz_elementqueries`.`id` = `blitz_elementquerysources`.`queryId`\n
    LEFT JOIN `blitz_elementqueryattributes` ON `blitz_elementqueries`.`id` = `blitz_elementqueryattributes`.`queryId`\n
    LEFT JOIN `blitz_elementqueryfields` ON `blitz_elementqueries`.`id` = `blitz_elementqueryfields`.`queryId`\n
    WHERE (`type`=:qp0) AND (NOT (0=1)) AND ((`sourceId` IS NULL) OR (`sourceId`=:qp1)) AND ((`attribute`=:qp2) OR (`attribute`=:qp3) OR (`fieldId`=:qp4))
    """
]

The only records in my table blitz_elementqueryattributes have attribute 'postDate'.
So the last where-condition:

->andWhere([
                    'or',
                    // Any date updated attributes should always be included
                    ['attribute' => ['dateUpdated']],
                    ['attribute' => $changedAttributes],
                    ['fieldId' => $changedFields],
                ]);

prevents any results.

@bencroker
Copy link
Collaborator

bencroker commented Jul 4, 2023

In my tests, $isChangedByAttributes is false, since the entry status has changed (which has higher priority than an attribute change). Please provide the exact steps you are using to test this, so I can try to replicate locally.

@JodebaDigitalPulse
Copy link
Author

JodebaDigitalPulse commented Jul 5, 2023

While writing this explanation I worked out that there is a setting for the Preparse fields parseBeforeSave and this solves all issues 😊

But this was my test case before: the Entrytype has a field of type Preparse Field
with this configuration:

contentColumnType: mediumtext
fieldGroup: d8403cb4-5695-4af7-8c43-f502a253d997 # Common
handle: f_preparse
instructions: null
name: Preparse
searchable: true
settings:
  allowSelect: false
  columnType: mediumtext
  decimals: 0
  displayType: textarea
  fieldTwig: '{% include ''_preparse/default'' %}'
  parseBeforeSave: false
  parseOnMove: false
  showField: false
  textareaRows: 10
translationKeyFormat: null
translationMethod: site
type: besteadfast\preparsefield\fields\PreparseFieldType

To avoid interference with other attributes, I just print this on a page:

<h1>{{ craft.entries.section('jobs').count() }}</h1>

When I disable one of the jobs entries, there isn't even a RefreshCacheJob, so nothing I can trace there.
The page doesn't refresh cache, count stay the same while it should be one less.

When I enable the job again, there is a RefreshCacheJob with this data:

{
    "description": null,
    "data": {
        "cacheIds": [],
        "elements": {
            "craft\\elements\\Entry": {
                "elementIds": {
                    "22173": true
                },
                "sourceIds": {
                    "6": true
                },
                "changedAttributes": {
                    "22173": {
                        "enabled": true
                    }
                },
                "changedFields": {
                    "22173": {
                        "f_preparse": true
                    }
                },
                "isChangedByAttributes": {
                    "22173": true
                },
                "isChangedByFields": {
                    "22173": false
                }
            }
        }
    },
    "forceClear": false,
    "forceGenerate": false
}

At this point the isChangedByAttributes is true. So there will be no tracked queries matched.

@bencroker
Copy link
Collaborator

Thanks, I'll look into what might be going on with the Preparse field.

@bencroker bencroker self-assigned this Jul 7, 2023
@bencroker
Copy link
Collaborator

I’m still unable to figure out what the issue is. In thinking through this, however, if your Preparse field contains an element query, then only the result of executing that query is stored in the database, not the query itself. Does that make sense?

@JodebaDigitalPulse
Copy link
Author

Hi Ben,

Sorry for the late reply.

I set up a test case without the Preparse field.

Please try this:
https://github.com/JodebaDigitalPulse/blitz-test-query

ddev start
ddev exec php craft install
ddev exec php craft migrate/all
  • Visit homepage, counter should state 10
  • Go to admin > entries > jobs:
  • Disable two entries (queue jobs will run)
  • counter still states 10
  • Enable one of the disabled entries (queue jobs will run)
  • counter will say 9

@bencroker
Copy link
Collaborator

Thanks, that was very helpful! I managed to narrow this down to tracked element queries ignoring disabled elements when determining which cached pages to refresh. Fixed in 95289cb with an accompanying feature test, and released in version 4.5.2.

@JodebaDigitalPulse
Copy link
Author

@bencroker thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants