[#24305] - [4.0] Fix Expected 'string' found 'int' error in com_content
- Closed
- 12 Mar 2020
- Medium
- Build: 4.0-dev
- # 24305
- Diff
- bahl24:expected-'string'-found-'int'
- Success Hound No violations found. Woof! Details
- Failure continuous-integration/drone/pr the build failed Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
User tests: Successful: Unsuccessful:
Pull Request for Issue - Fixes Expected 'string' found 'int' error in com_content. .
Summary of Changes
Changed to string
Expected result
string type
Actual result
int type
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_content |
rdeutz
- | Labels |
Added:
?
|
||
I too think that this is not the right approach and we should rather fix the method than to mask this like this.
When I writing this method I think state value will be loaded from SQL so it will often be string.
But for real use, I agree it should remove string type hint.
If change the addState() signature, we should also modify getState()
https://github.com/joomla/joomla-cms/pull/18501/files#diff-72e4f228b49d04896cfdffaa98f94045R131
We have workflow system now, maybe this class should also rewrite to fit workflow.
@asika32764 Are you working on this or should I try to modify the addState() and other functions in a separate PR as there are many other functions with such type hints?
Since I'm joining some serious projects I'm afraid of I will unable to continue work with this function. If someone want to modify this method, just try to pass the test, then I think it will work fine.
Ok, let me try
| Category | Administration com_content | ⇒ | Administration com_content Libraries |
| Category | Administration com_content Libraries | ⇒ | Libraries |
| Title |
|
||||||
franz-wohlkoenig
- | Title |
|
||||||
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-03-12 16:51:08 |
| Closed_By | ⇒ | wilsonge | |
| Labels |
Added:
?
|
||
No action here and I'm still yet to here of anyone else getting the original issue so I'm closing this for now. It can be reopened if we reproduce this in the future.
This doesn't cause a real issue now because we don't use strict_types directive. I think this should be changed anyways. We use only integer states (only passed as string if from DB). Do we want to support non-integer states here (for 3rd parties)?
We should typehint to an integer earlier i assume (and typehint method accordingly)
Looks like this won't work because internally Joomla\CMS\Button\ActionButton uses _default as some sort of default state.
Ugh and that is typehinted to string
Judging by the PR that added button classes (#18501), value typehints were added by mistake and should be removed. @asika32764?