[#15331] - Added a getSaveData function in form controller for data src override
- Closed
- 22 May 2020
- Medium
- Build: staging
- # 15331
- Diff
- julienV:getsavedata
- Success hound No violations found. Woof! Details
- Success continuous-integration/drone/pr the build was successful 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:
Summary of Changes
This is a very simple change to allow for form controller subclasses to easily override the source of data.
Currently, you have to rewrite the full save function if you want to add data other than jform post data.
For example for file uploads, a commonly found workaround, is to use the prepareTable function of the model, but i think it would be much cleaner to pull the data from controller, and not the model.
Testing Instructions
It shouldn't change anything as long as there is no override, so testing is just saving any form using the regular form controller (e.g: an article)
Expected result
works as usual
Actual result
works...
Documentation Changes Required
Changes nothing for end user
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15331.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15331.
3rd party developer using this function name already may have a b/c break, also reading the code correctly this could be done in the jmodel->validate function, actually I think it's the correct place.
I don't think so, it's the controller role to get the data from input, not the model's.
I know joomla models are already doing it from the populatestate function, doesn't make it right though.
see the original description above too.
There is another way to modify the data with a event, I'm not sure if we really need 3 ways to do the same thing.
@HLeithner What shall we do with this PR? Remove RTC? Set any other label?
you have to be more explicit about what your solution is @HLeithner
again, for me the right place to get the data from input is the controller, not the model. You know, as in MVC architecture...
Beside the inconsistency in this PR ($recordId = $this->input->getInt($urlVar); is still loaded from JInput) I see not a good reason to add this to core, in J3 we could have a b/c break with 3rd party extensions (using the function name). For feature use we could have 100s of such functions and this would reduce performance without a good reason. If you like to modify the input then you can override the save function and change the input value or do any other of the options already said.
Which maybe makes for sense to split this 300 lines function into more logical parts to get better readability and reusability.
I'm closing this for now, thx for your engagement.
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-05-22 10:01:12 |
| Closed_By | ⇒ | HLeithner | |
| Labels |
Added:
?
|
||
that was the point that it's a shame to have to override the whole controller save function for just the input... but ok, whatever.
I have tested this item✅ successfully on f16ab9d
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15331.