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.3] Fix code logic in admin controllers #45037

Merged
merged 4 commits into from
Feb 28, 2025

Conversation

joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

In some of our controller classes, we have this line of code $recordId = (int) isset($data[$key]) ? $data[$key] : 0; . It is clearly we want to have int casting applied to $data[$key]. However, with the current code, int casting is actually being applied to isset($data[$key]) and it is not right. This PR fixes that wrong logic.

Testing Instructions

The change is the same for all controllers, so we will just need to test one

  1. Use Joomla 5.3
  2. Try to edit a banner, save it and make sure it is still working.

Actual result BEFORE applying this Pull Request

Works, but int casting is not applied to $data[$key] as expected

Expected result AFTER applying this Pull Request

Works, and int casting is applied properly to $data[$key] as expected

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

Sorry, something went wrong.

@QuyTon
Copy link
Contributor

QuyTon commented Feb 28, 2025

I have tested this item ✅ successfully on 0fbf20c


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

@richard67
Copy link
Member

@joomdonation You should restore @QuyTon 's test result in the issue tracker because it got lost with the branch update. You can use the "Alter test" button for that.

@joomdonation
Copy link
Contributor Author

Thanks @richard67. I had to don that to trigger CI re-run. I now altered test result from @QuyTon

@richard67
Copy link
Member

I have tested this item ✅ successfully on c5277db


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 28, 2025
@rdeutz rdeutz enabled auto-merge (squash) February 28, 2025 16:58
@rdeutz rdeutz merged commit e016edc into joomla:5.3-dev Feb 28, 2025
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 28, 2025
@richard67 richard67 added this to the Joomla! 5.3.0 milestone Mar 1, 2025
@joomdonation joomdonation deleted the fix_code_logic branch March 1, 2025 11:32
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.

None yet

5 participants