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

[libraries][installer] - ModuleAdapter #22984

Merged
merged 2 commits into from
Dec 5, 2018
Merged

[libraries][installer] - ModuleAdapter #22984

merged 2 commits into from
Dec 5, 2018

Conversation

alikon
Copy link
Contributor

@alikon alikon commented Nov 9, 2018

Pull Request for a partial fix to Issue #22949 .

Summary of Changes

cast to int for postgresql

Testing Instructions

see #22949

@twister65
Copy link
Contributor

It fixes plugin update issue.
Very strange, the changes are in ModuleAdapter.
The module update issue is still there.

@twister65
Copy link
Contributor

Please, replace $db->quote by $db->quoteName

->where($db->quote('client_id') . ' = ' . (int) $this->extension->client_id);

->where($db->quoteName('client_id') . ' = ' . (int) $this->extension->client_id);

Copy link
Contributor

@twister65 twister65 left a comment

Choose a reason for hiding this comment

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

I think, this line :
joomla-cms/libraries/src/Installer/Adapter/ModuleAdapter.php

Line 701 in 1ea6ca6

	$this->extension->delete($this->extension->extension_id); 

should be placed after the query (line 717), when we have finished using it.
It is better to delete the object, when we no longer need it.

@mbabker
Copy link
Contributor

mbabker commented Nov 9, 2018

@twister65 That should really be a separate pull request instead of lumped in with another unrelated change.

@twister65
Copy link
Contributor

See (test) PR #22993 please.

Co-Authored-By: alikon <optimus4joomla@gmail.com>
@csthomas
Copy link
Contributor

csthomas commented Dec 3, 2018

I have tested this item ✅ successfully on 3617699

Code review


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

@Quy
Copy link
Contributor

Quy commented Dec 3, 2018

I have tested this item ✅ successfully on 3617699


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

@joomla-cms-bot joomla-cms-bot changed the title [libraries][installer] - ModuleAdapter [libraries][installer] - ModuleAdapter Dec 3, 2018
@Quy
Copy link
Contributor

Quy commented Dec 3, 2018

RTC


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

@joomla-cms-bot joomla-cms-bot added RTC This Pull Request is Ready To Commit labels Dec 3, 2018
@mbabker mbabker added this to the Joomla 3.9.2 milestone Dec 5, 2018
@mbabker mbabker merged commit 18dd556 into joomla:staging Dec 5, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 5, 2018
@alikon alikon deleted the patch-109 branch December 5, 2018 07:56
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.

None yet

6 participants