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

Correcting SQL for required_options column #338

Closed
wants to merge 2 commits into from
Closed

Correcting SQL for required_options column #338

wants to merge 2 commits into from

Conversation

parhamr
Copy link

@parhamr parhamr commented Aug 21, 2013

Changes

  • Replaces string “1” with integer 1, as the required_options column is smallint(5) unsigned

Testing

  • Obvious change; only a code review is necessary
mysql> describe catalog_product_entity;
+------------------+----------------------+------+-----+---------+----------------+
| Field            | Type                 | Null | Key | Default | Extra          |
+------------------+----------------------+------+-----+---------+----------------+
| entity_id        | int(10) unsigned     | NO   | PRI | NULL    | auto_increment |
| entity_type_id   | smallint(5) unsigned | NO   | MUL | 0       |                |
| attribute_set_id | smallint(5) unsigned | NO   | MUL | 0       |                |
| type_id          | varchar(32)          | NO   |     | simple  |                |
| sku              | varchar(64)          | YES  | MUL | NULL    |                |
| created_at       | timestamp            | YES  |     | NULL    |                |
| updated_at       | timestamp            | YES  |     | NULL    |                |
| has_options      | smallint(6)          | NO   |     | 0       |                |
| required_options | smallint(5) unsigned | NO   |     | 0       |                |
| erp_arrival_date | datetime             | YES  |     | NULL    |                |
+------------------+----------------------+------+-----+---------+----------------+

… the value passed to ORM does not need to be string.
@verklov
Copy link
Contributor

verklov commented Nov 7, 2013

Hello parhamr,
Sorry for the delay with the response.
Thank you for your contribution! Our team will review your pull request and respond as soon as our analysis is complete.

magento-team added a commit that referenced this pull request Nov 29, 2013
* Modularity improvements:
  * Breakdown of the Adminhtml module:
     * Moved Newsletter, Report logic to the respective modules
     * Moved blocks, config, view, layout files of other components from Adminhtml folder to respective modules
  * Removed application dependencies from the library
* Move Magento\Core common blocks in the library
* Application areas rework:
  * Areas are independent from Store
  * Removed deprecated annotation from the getArea methods
* GitHub requests:
  * [#245](#245) -- Resolve design flaws in core URL helper
  * [#247](#247) -- Bug in Mage_Page_Block_Html_Header->getIsHomePage
  * [#259](#259) -- Turkish Lira (TRY) is supported for Turkish members.
  * [#262](#262) -- Update Rule.php
  * [#373](#373) -- [Magento/Sales] Fixed typos
  * [#382](#382) -- [Magento/Core] Fixed typos
  * [#304](#304) -- Removed Erroneous closing "
  * [#323](#323) -- InstanceController.php - made setBody protected
  * [#349](#349) -- Move Mage_Catalog menu declaration into Mage_Catalog module.
  * [#265](#265) -- Update Merge.php
  * [#271](#271) -- Check Data should validate gallery information
  * [#305](#305) -- Extra ", tidied up nested quotes
  * [#352](#352) -- Add Croatia Country as part of European Union since 1st July 2013 for default european local countries in configuration
  * [#224](#224) -- Tax formatting is locale aware and should not
  * [#338](#338) -- Correcting SQL for required_options column
  * [#327](#327) -- cart api bug fix & partial invoice credit memo divide by zero warning
* Themes update:
  * Old frontend (magento_demo) and backend (magento_basic) themes are removed
  * Updated templates and layout updates in the Bundle, Catalog, CatalogInventory, CatalogSearch, Downloadable, ProductAlert, Reports, Sendfriend modules
* Fixed bugs:
  * Fixed the error when  Magento cannot be reinstalled to the same database with table prefix
  * Fixed report Products in Cart
  * Fixed error on attempt to insert image to CMS pages under version control
  * Fixed order status grid so that you can assign state, edit, and view custom order status
  * Fixed Related Products Rule page so that category can be selected on conditions tab
  * Fixed Magento_Paypal_Controller_ExpressTest integration test so it is re-enabled
  * Fixed the bug with international DHL quotes
@elenleonova
Copy link

Dear parhamr,
We have processed your pull request!
Please, continue reviewing our code.

@parhamr
Copy link
Author

parhamr commented Nov 29, 2013

@eleonova thanks! I appreciate the encouragement.

vpelipenko added a commit that referenced this pull request Jun 5, 2015
[Nord] M2 GitHub Update (version 0.74.0-beta11)
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.

3 participants