Skip to content

Import from database dump only imports first table#22660

Merged
HLeithner merged 1 commit intojoomla:stagingfrom
gslitt91:staging
Aug 19, 2019
Merged

Import from database dump only imports first table#22660
HLeithner merged 1 commit intojoomla:stagingfrom
gslitt91:staging

Conversation

@gslitt91
Copy link
Contributor

@gslitt91 gslitt91 commented Oct 16, 2018

Import from database dump only imports first table .

Pull Request for Issue #22620.

Summary of Changes

On expression inside foreach

Testing Instructions

  1. Export structure dump for more then one table.
  2.  Merge from dump generated in step one .

Expected result

  it shall merge all tables .

Actual result

it only merges first table

Documentation Changes Required

Import from database dump only imports first table .
@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
Copy link
Contributor

@zero-24 zero-24 left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@richard67
Copy link
Member

@mbabker Shouldn't this change not be also made in the framework? It seems in the framework there is also a typo in the function name "getAlterTableql", but in the CMS (J3) there is not this typo. See here the framework: https://github.com/joomla-framework/database/blob/master/src/DatabaseImporter.php#L264

@mbabker
Copy link
Contributor

mbabker commented Aug 19, 2019

Every change here should eventually make it to the Framework package if the same issue exists in a similar block of code. That goes for all Framework packages, not just the database.

@richard67
Copy link
Member

@mbabker I don't dare to ask how it comes that it is different now regarding that typo I've mentioned.

@mbabker
Copy link
Contributor

mbabker commented Aug 19, 2019

Someone screwed up copy/pasting something, code that never had test coverage to detect the typo in the first place, someone mistyped it when creating it, who knows.

@richard67
Copy link
Member

Well, I will care and make PR's against framework db package sooner or later. The typo is already fixed there in 2.0-dev, the change from this PR neither in master nor in 2.0-dev.

@HLeithner
Copy link
Member

thx richard for your help, I will merge this into core, please take care bringing this upstream as discussed on glip.

and many thx to @gslitt91 for creating this PR and make Joomla a bit database import friendly!

also a thank you to @mbabker for the helpful feedback.

@richard67
Copy link
Member

@HLeithner The correction from this PR and the typo I've mentioned in our Glip discussion have been merged now in the framework database package in both the master branch and the 2.0-dev branch. As soon as a composer update is made to the latest version of joomla/database, these changes are in 4.0-dev branch here. I've pinged Brian to update his PR #25840 for updating dependencies. If he will not do it, I will do a PR later after his PR has been merged. An update will fetch us also other recent changes e.g. for PHP 7.4 (array curly brackets).

@HLeithner
Copy link
Member

Thanks Richard for managing this patch.

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.

6 participants