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

[5.0] Fix legacy table creation #42180

Merged
merged 3 commits into from
Nov 18, 2023

Conversation

HLeithner
Copy link
Member

Pull Request for Issue #42179 .

Summary of Changes

If a legacy table is created by Table::getInstance() twice it fails the second time. Reason for this is that the legacy table name is not used if the table class already exists.

Testing Instructions

Test joomla core and a random 3rd party extension which uses tables

Actual result BEFORE applying this Pull Request

Could fail if Table::getInstance() is called more then 1 time for the same old (JTableXXX) syntax.

Expected result AFTER applying this Pull Request

works

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

If a legacy table is created by Table::getInstance() twice it fails the second time. Reason for this is that the legacy table name is not used if the table class already exists.
@laoneo
Copy link
Member

laoneo commented Oct 20, 2023

Did you change something in the code? Only intendation is added.

@Quy
Copy link
Contributor

Quy commented Oct 20, 2023

Add w=1 to hide whitespace changes. https://github.com/joomla/joomla-cms/pull/42180/files?w=1

@laoneo
Copy link
Member

laoneo commented Oct 20, 2023

Thanks, I was sure that I miss here something :-)

@HLeithner
Copy link
Member Author

@georgebara can you please test this pr?

@georgebara
Copy link

Thank you for the fast feedback.

@HLeithner HLeithner added the bug label Oct 22, 2023
@richard67
Copy link
Member

@georgebara Was your approval a real successful test? If so, please mark your test result so it's properly counted. For doing this, go to the PR in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/42180 , then use the blue "Test this" button at the top left corner, then select your test result and finally submit. Thanks in advance.

@georgebara
Copy link

I have tested this item ✅ successfully on 6265a1b

The issue could not be reproduced anymore.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42180.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 6265a1b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42180.

@Quy Quy removed the bug label Nov 16, 2023
@Quy
Copy link
Contributor

Quy commented Nov 16, 2023

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42180.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 16, 2023
@bembelimen bembelimen merged commit 5e9aea0 into joomla:5.0-dev Nov 18, 2023
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 18, 2023
@bembelimen bembelimen added this to the Joomla! 5.0.1 milestone Nov 18, 2023
@bembelimen
Copy link
Contributor

Thx

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.

9 participants