- Merge request !9Issue #3188203: PHP Exception on overview / Add default config β (Merged) created by batkor
- π©πͺGermany skrug Bamberg
I provide a patch file containing changes of the branch. Worked for me.
- First commit to issue fork.
- π¬π§United Kingdom alexpott πͺπΊπ
This doesn't fix the problem. I think the problem requires a different solution. The view is assuming that nodes are configured to use the lock when they are not. Hence the problem. I think we should only grant access to the view when node is in the lockable entity types.
- π¬π§United Kingdom alexpott πͺπΊπ
Also I think that this clear needs to not be in the form. In needs to be in \Drupal\content_lock\EventSubscriber\SettingsSaveEventSubscriber::onSave
With something like
if ($event->isChanged('types') && \Drupal::moduleHandler()->moduleExists('views')) { Views::viewsData()->clear(); }
- πͺπΈSpain penyaskito Seville π, Spain πͺπΈ, UTC+2 πͺπΊ
Clearing views data is not enough. If no content is enabled, this still would fail with
Drupal\Core\Database\DatabaseExceptionWrapper: Exception in Locked content[locked_content]: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'timestamp' in 'field list': SELECT COUNT(*) AS "expression" FROM (SELECT DISTINCT "node_field_data"."langcode" AS "node_field_data_langcode", "users_field_data_content_lock"."langcode" AS "users_field_data_content_lock_langcode", "node_field_data"."nid" AS "nid", "users_field_data_content_lock"."uid" AS "users_field_data_content_lock_uid", timestamp AS "timestamp", langcode AS "langcode", 1 AS "expression" FROM "node_field_data" "node_field_data" INNER JOIN "users_field_data" "users_field_data_content_lock" ON uid = users_field_data_content_lock.uid WHERE ""."timestamp" <> :db_condition_placeholder_0) "subquery"; Array ( [:db_condition_placeholder_0] => 0 ) in main() (line 19 of index.php).
We probably need to add the columns no matter if the content is enabled or not. Another alternative might be having a controller that takes the view url when no content is enabled, and renders the view itself when some content is enabled?
We probably want to check how content moderation handles this in core, as it's a similar scenario. - πͺπΈSpain penyaskito Seville π, Spain πͺπΈ, UTC+2 πͺπΊ
@alexpott I wrote #11 without seeing your #9 and #10 and came to similar conclusion. Will work on this tomorrow.
- πΊπΈUnited States smustgrave
β¨ Add tasks links to add Locked content view to /admin/content tabs RTBC is blocked on this one.
- πΊπΈUnited States smustgrave
Question, the current issue summary mentions shipping with default config having nodes enabled. Is that a viable approach?
- π¬π§United Kingdom alexpott πͺπΊπ
@smustgrave then the module will have to depend on node and you can still get into this situation by removing node configuration.
- Status changed to Needs review
6 months ago 9:17pm 26 March 2024 - πͺπΈSpain penyaskito Seville π, Spain πͺπΈ, UTC+2 πͺπΊ
So I've added a custom views access plugin that can check if any types are configured.
This custom views access plugin also adds a custom access checker to the route, so when no content type is configured will return an Access denied.
As we are adding a new method to the interface this might not be backwards compatible.
- Status changed to Needs work
6 months ago 10:53pm 26 March 2024 - πΊπΈUnited States smustgrave
So manually tested and when I go to the view page I get an access denied.
Tried adding a test for it but get
Drupal\Core\Config\Schema\SchemaIncompleteException : Schema errors for views.view.locked_content with the following errors: views.view.locked_content:display.default.display_options.access.options missing schema
- πΊπΈUnited States smustgrave
When I open the view I see
SQLSTATE[42S22]: Column not found: 1054 Unknown column 'timestamp' in 'field list': SELECT COUNT(*) AS "expression" FROM (SELECT DISTINCT "node_field_data"."langcode" AS "node_field_data_langcode", "users_field_data_content_lock"."langcode" AS "users_field_data_content_lock_langcode", "node_field_data"."nid" AS "nid", "users_field_data_content_lock"."uid" AS "users_field_data_content_lock_uid", timestamp AS "timestamp", langcode AS "langcode", 1 AS "expression" FROM "node_field_data" "node_field_data" INNER JOIN "users_field_data" "users_field_data_content_lock" ON uid = users_field_data_content_lock.uid WHERE ""."timestamp" <> :db_condition_placeholder_0) "subquery"; Array ( [:db_condition_placeholder_0] => 0 )
- Status changed to Needs review
6 months ago 11:22pm 26 March 2024 - πΊπΈUnited States smustgrave
I see the access denied was intentional.
I added a simple test (can be expanded on later) that checks this.
For the schema issue since this access check doesn't need options I removed that from the view.
Thoughts?
- Status changed to RTBC
5 months ago 1:41am 27 March 2024 - πΊπΈUnited States smustgrave
Removed the pagination_header line and tests are green. Since I just added the tests I think it's fine to mark.
Once this lands I'll expand on the tests in π Extend test coverage Needs work which has a last step to add views integration testing.
- π¬π§United Kingdom alexpott πͺπΊπ
We could file a follow-up to make the new views access plugin configurable.
-
alexpott β
committed 0f9476a1 on 8.x-2.x authored by
batkor β
Issue #3188203 by alexpott, penyaskito, smustgrave, batkor, skrug,...
-
alexpott β
committed 0f9476a1 on 8.x-2.x authored by
batkor β
- Status changed to Fixed
5 months ago 12:46pm 27 March 2024 Automatically closed - issue fixed for 2 weeks with no activity.