Problem/Motivation
Views "Global: Custom text" field handler should not be sortable because there is no SQL column to ORDER.
Steps to reproduce
See the original report.
Proposed resolution
Disable click sorting on "Global: Custom text" field handler
Remaining tasks
Commit the patch.
User interface changes
Before
After
API changes
None
Data model changes
None
Release notes snippet
N/A
Original report by @dpearcefl
- Add a Global: custom text field to a table display View
- Select my custom field as sortable.
- But if I click on that column or make it the default sort, I get this error message:
SQLSTATE[42S22]: Column not found: 1054 Unknown column 'unknown' in 'order clause': SELECT node.nid AS nid, node_field_data.title AS node_field_data_title FROM {node} node INNER JOIN {node_field_data} node_field_data ON node.nid = node_field_data.nid WHERE (( (node_field_data.status = :db_condition_placeholder_0) AND (node_field_data.type IN (:db_condition_placeholder_1)) )) ORDER BY unknown ASC LIMIT 50 OFFSET 0; Array ( [:db_condition_placeholder_0] => 1 [:db_condition_placeholder_1] => sample )
I looked but didn't see this mentioned in another issue. Just wanted to mention it.
Comment | File | Size | Author |
---|---|---|---|
#27 | 2223195-27.patch | 1.53 KB | Suresh Prabhu Parkala |
#22 | 2223195-22.patch | 1.45 KB | sulfikar_s |
#17 | 2223195-17.patch | 1.53 KB | Lendude |
#17 | interdiff-2223195-16-17.txt | 406 bytes | Lendude |
#16 | interdiff_11_16.txt | 514 bytes | anmolgoyal74 |
Comments
Comment #1
LendudeMoving to the right queue.
Updating the title and Issue summary to reflect the actual bug.
On a side note, the original post was for entity references not being sortable, but that works now.
Comment #10
artmarks7 CreditAttribution: artmarks7 commentedI've also just run into this error.
Drupal\Core\Database\DatabaseExceptionWrapper: Exception in Projects[projects]: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'unknown' in 'order clause': SELECT node_field_revision.vid AS vid, node_field_data_node_field_revision.nid AS node_field_data_node_field_revision_nid, users_field_data_node_field_revision.uid AS users_field_data_node_field_revision_uid, content_moderation_state_field_data_node_field_data.id AS content_moderation_state_field_data_node_field_data_id, content_moderation_state_field_revision_node_field_revision.revision_id AS content_moderation_state_field_revision_node_field_revision_ FROM {node_field_revision} node_field_revision LEFT JOIN {node_field_data} node_field_data_node_field_revision ON node_field_revision.nid = node_field_data_node_field_revision.nid AND node_field_data_node_field_revision.langcode = node_field_revision.langcode LEFT JOIN {users_field_data} users_field_data_node_field_revision ON node_field_revision.uid = users_field_data_node_field_revision.uid LEFT JOIN {content_moderation_state_field_data} content_moderation_state_field_data_node_field_data ON node_field_data_node_field_revision.nid = content_moderation_state_field_data_node_field_data.content_entity_id AND content_moderation_state_field_data_node_field_data.content_entity_type_id = :views_join_condition_1 LEFT JOIN {content_moderation_state_field_revision} content_moderation_state_field_revision_node_field_revision ON node_field_revision.vid = content_moderation_state_field_revision_node_field_revision.content_entity_revision_id AND content_moderation_state_field_revision_node_field_revision.content_entity_type_id = :views_join_condition_2 LEFT JOIN {node_field_revision} node_field_revision2 ON node_field_revision.nid = node_field_revision2.nid AND node_field_revision2.vid > node_field_revision.vid INNER JOIN {node__field_project_director_area} node_field_data_node_field_revision__node__field_project_director_area ON node_field_data_node_field_revision.nid = node_field_data_node_field_revision__node__field_project_director_area.entity_id AND node_field_data_node_field_revision__node__field_project_director_area.deleted = :views_join_condition_4 WHERE ((node_field_data_node_field_revision__node__field_project_director_area.field_project_director_area_target_id = :node__field_project_director_area_field_project_director_area_target_id)) AND ((node_field_revision2.nid IS NULL) AND (node_field_data_node_field_revision.type IN (:db_condition_placeholder_5))) ORDER BY unknown ASC LIMIT 50 OFFSET 0; Array ( [:node__field_project_director_area_field_project_director_area_target_id] => 494 [:db_condition_placeholder_5] => project [:views_join_condition_1] => node [:views_join_condition_2] => node [:views_join_condition_4] => 0 ) in Drupal\views\Plugin\views\query\Sql->execute() (line 1543 of /code/web/core/modules/views/src/Plugin/views/query/Sql.php).
Anybody got an idea for a fix?
Comment #11
Lendude@artmarks7 was that also with a Custom text field?
Just looking at it, I'd say the custom text field shouldn't be click sortable at all, it is not part of the query so it won't work.
Comment #12
LendudeReally not sure if this needs a test, we are just changing a setting after all, not updating any logic
Also, doing an upgrade path for this feels very excessive, since the upgrade would do the same as a manual fix, just remove the click sorting. It's not like it would suddenly start to work.
So, just some before and after screenshots for now
Before
After
Comment #13
larowlanAgree, not sure on how to test this anyway, we'd be just testing the views_data (kernel) or the UI
Comment #14
jibranThis is a logical change so I don't think we need tests here. The sortable setting option is already has tests in
\Drupal\Tests\views\Kernel\Handler\FieldKernelTest::testClickSortable()
. Given that I'm going to set it to RTBC. Also updated IS and added screenshots to IS as well.Comment #15
alexpottThe fix looks fine but I think we need to update existing views - no? And we also need a post update function to clear the cache.
Comment #16
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedAdded post update function to clear the cache.
Comment #17
LendudeThis updates the existing Views in core, only 4 use this plugin and only 1 of those uses the table style, so updated that.
Since we still have #2917814: Views table style info never gets updated when changing/adding fields, I don't think we should bother writing an update hook to do this in project config, the data in that part of the configuration can be hopelessly out of sync anyway, so we would probably be updating garbage. But that is just my 2ct...
Comment #18
alexpott@Lendude we still need the empty post update to clear the cache so the change is available.
Comment #19
alexpottComment #20
Lendude@anmolgoyal74 added the empty post update in #16, that should do the trick right?
Comment #21
alexpott@Lendude @anmolgoyal74 oops I missed that. Because we need an update this is 9.2.x only. #2917814: Views table style info never gets updated when changing/adding fields looks nice and tricky but I agree if we're already out of sync then this is already a mess.
Comment #22
sulfikar_s CreditAttribution: sulfikar_s at Zyxware Technologies commentedHi, the patch in #17 didn't apply on 9.2.x-dev, so I've rerolled it.
Please review.
Comment #23
larowlanSetting back to RTBC, I'm not sure if @alexpott's comment in #21 is agreeing with #17, i.e. that no update hook is fine - deferring to @alexpott to confirm that's the case
Comment #24
dwwReroll in #22 looks good.
I agree we do not need an update hook given how complicated / messy #2917814: Views table style info never gets updated when changing/adding fields makes this.
No code style problems.
Also agree there's nothing really to add a test for here, and all existing tests pass.
+1 to RTBC. Thanks, everyone!
Comment #26
catchCommitted/pushed to 9.2.x, thanks!
This doesn't cherry-pick cleanly to 9.1.x.
Comment #27
Suresh Prabhu Parkala CreditAttribution: Suresh Prabhu Parkala at Specbee commentedRe-rolled the patch against 9.1.x. Plaese review
Comment #29
catchAgreed that we can add the update hook in #2917814: Views table style info never gets updated when changing/adding fields - it'll cover the change made here but more generically, and no point trying to update the data without that issue being fixed. If the order had been different we'd need to re-run similar logic here too though.
For patches that are only going to land in a minor, I think we can skip empty post updates since there's an extremely high likelihood that there'll be another core update or something to clear the cache. However since this is going into 9.1.x too the empty post update is good.
If we tried to backport this to 8.9.x we'd have out-of-sync post updates with 9.0.x to worry about, but since this is 'normal' we can skip the backport, and hence not worry about that.
The 9.1.x patch looks good so have committed/pushed to 9.1.x too. Thanks everyone.