Skip to content

Conversation

@magently
Copy link

Description

lib/internal/Magento/Framework/Model/ResourceModel/Db/Collection/AbstractCollection.php
addExpressionFieldToSelect() method added it's result to getSelect()->columns. When you called addFieldToSelect() method after addExpressionFieldToSelect() expressions where removed from columns. Your select was overwriten by addFieldToSelect(). To avoid that i changed the addExpressionFieldToSelect() method to insert it's result into $this->_fieldsToSelect array as \Zend_Db_Expr. This way it does not get overwritten and you get results from both methods in your collection no matter the order you call them in.

Fixed Issues (if relevant)

  1. addExpressionFieldToSelect has to be called after all addFieldToSelect #17635

Manual testing scenarios

  • create a collection
  • before the change the following code will only show finish_date in collection:
    $bookingCollection = $this->_bookingCollectionFactory->create();
    $bookingCollection->addExpressionFieldToSelect('stime_part', 'time(start_time)', []);
    $bookingCollection->addFieldToSelect('finish_time', 'finish_date');
    -after proposed change you will see both stime_part and finish_date

@magento-engcom-team
Copy link
Contributor

Hi @magently. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@okorshenko
Copy link
Contributor

Hi @magently
Thank you for your contribution and welcome to Magento Community!
It would be great if you can cover your improvements with the integration test.
Thank you

@sidolov
Copy link
Contributor

sidolov commented Sep 6, 2018

Hi @magently , I'm closing this PR as duplicate for #17915

@sidolov sidolov closed this Sep 6, 2018
@magently magently deleted the fix/addExpression branch August 9, 2019 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants