Skip to content

[MI-1705 Fix an incorrect assumption that a core_store_group with ID 1 always exists. #128

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 21, 2017
Merged

[MI-1705 Fix an incorrect assumption that a core_store_group with ID 1 always exists. #128

merged 1 commit into from
Jun 21, 2017

Conversation

samgavinio
Copy link
Contributor

@samgavinio samgavinio commented Jun 20, 2017 •
edited
Loading

/cc @zendesk/mintegrations

Description

  • Fix an incorrect assumption that a core_store_group with ID 1 always exists.
  • Fetch store and website from current configuration scope rather than making assumptions based on ordering

screen shot 2017-06-21 at 1 25 21 am

Steps to reproduce bug:

  • Access the mysql database then execute the following query against the magento database:
UPDATE core_store_group SET group_id = 99 WHERE group_id = 1

References

Risks

  • [low] Passed name to zendesk_provisioning may still not be the name of the currently being configured scope
  • [medium] Could generate zendesk/api link for the incorrect store

@samgavinio
Fetch store and website from current configuration scope.
ba507e3 
This also fixes an incorrect assumption that a core_store_group with
ID always exists.
samgavinio
samgavinio commented Jun 20, 2017
View reviewed changes
src/app/code/community/Zendesk/Zendesk/Block/Adminhtml/Config/Buttons/Signup.php
'magento_callback' => Mage::helper('adminhtml')->getUrl('adminhtml/zendesk/redirect', array('type' => 'settings', 'id' => 'zendesk')),
'magento_locale' => Mage::getStoreConfig('general/locale/code'),
'magento_timezone' => Mage::getStoreConfig('general/locale/timezone'),
'magento_api_url' => Mage::getUrl('zendesk/api', array('_store' => $stores[1]->getDefaultStore()->getCode()))
Copy link
Contributor Author

@samgavinio samgavinio Jun 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note on this fix for https://support.zendesk.com/agent/tickets/2554327

Using $stores[1] here is an incorrect assumption since the indexes here are the database primary key. This will throw a fatal error if no core_store_group with primary key 1 exists:

Call to a member function getDefaultStore() on null

@samgavinio samgavinio merged commit a0970ba into master Jun 21, 2017
@samgavinio samgavinio deleted the samgavinio/MI-1571-fix-error-store-id branch June 21, 2017 04:48
@samgavinio samgavinio changed the title [MI-1571] Fix an incorrect assumption that a core_store_group with ID 1 always exists. [MI-1705 Fix an incorrect assumption that a core_store_group with ID 1 always exists. Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@kcasas kcasas kcasas approved these changes

@jmramos02 jmramos02 jmramos02 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@samgavinio @kcasas @jmramos02