Skip to content

[4.0] loadmodulebyid fix strict comparison#28269

Closed
alikon wants to merge 1 commit intojoomla:4.0-devfrom
alikon:patch-109
Closed

[4.0] loadmodulebyid fix strict comparison#28269
alikon wants to merge 1 commit intojoomla:4.0-devfrom
alikon:patch-109

Conversation

@alikon
Copy link
Contributor

@alikon alikon commented Mar 8, 2020

Pull Request for Issue # .

Summary of Changes

cast to int to fix strict comparison

Testing Instructions

create/edit an article
click on CMS Content and select
select a module to embed in the article
save the article

Expected result

the module output is showed in the article

Actual result

no module output

@infograf768
Copy link
Member

Please test first
#28259

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Mar 8, 2020

There's already PR for staging #26531. But it's also wrong.

@alikon
Copy link
Contributor Author

alikon commented Mar 8, 2020

why this is wrong ?

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Mar 8, 2020

Read the comments in #26531.

@alikon
Copy link
Contributor Author

alikon commented Mar 8, 2020

umm i was testing with postgresql....i'll look with mysql .....

@alikon
Copy link
Contributor Author

alikon commented Mar 8, 2020

work with mysql as well

@chmst
Copy link
Contributor

chmst commented Mar 8, 2020

I have tested this item ✅ successfully on b12efc0

on code inspect


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

@jwaisner
Copy link
Member

jwaisner commented Mar 8, 2020

I have tested this item ✅ successfully on b12efc0

Tested with adding CMS content to article. Content displays correctly after PR is applied.


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

@jwaisner
Copy link
Member

jwaisner commented Mar 8, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 8, 2020
@SharkyKZ
Copy link
Contributor

SharkyKZ commented Mar 8, 2020

No RTC. This breaks PDO.

@brianteeman brianteeman added PR-4.0-dev and removed RTC This Pull Request is Ready To Commit labels Mar 8, 2020
@jwaisner
Copy link
Member

jwaisner commented Mar 8, 2020

Set back to Pending.


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

@alikon
Copy link
Contributor Author

alikon commented Mar 9, 2020

@SharkyKZ may i ask you to test on PDO please ?
i've tested with all the 3 flavour of db's supported on j4 and it works as expected with this pr on all the 3

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Mar 9, 2020

PDO returns strings for numeric columns when using pdo_mysql.

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Mar 9, 2020

This seems to be because the driver is using PDO::ATTR_EMULATE_PREPARES.

@alikon
Copy link
Contributor Author

alikon commented Apr 15, 2020

closing as this will be probably fixed when #28278 will land in 4 branch

@alikon alikon closed this Apr 15, 2020
@alikon alikon deleted the patch-109 branch April 15, 2020 06:52
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.

7 participants