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

[4.0] Fixed lost request parameters on save #26675

Merged
merged 1 commit into from
Dec 6, 2019

Conversation

HRIT-Florian
Copy link
Contributor

Pull Request for Issue #26657.

Summary of Changes

On menu save, reloading form request values missing on error.
Screenshot, see issue #26657

Testing Instructions

See issue #26657

Expected result

See issue #26657

Actual result

See issue #26657

Documentation Changes Required

Nope

@Fallatas
Copy link

I have tested this item ✅ successfully on 08f6579


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

@joomlamarco
Copy link

I have tested this item ✅ successfully on 08f6579


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

@SharkyKZ
Copy link
Contributor

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 20, 2019
@HLeithner
Copy link
Member

If I checked the history correctly this code exists since 10+ years. Why does it fail now and does it impact any other component?

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Dec 6, 2019

It fails in 3.x too. My guess the idea here is not to send request to the model because request doesn't exist in the table and is not stored by the model.

If you want, this can be refactored so request is still stored to user state but not sent to the model.

@HRIT-Florian
Copy link
Contributor Author

HRIT-Florian commented Dec 6, 2019

@HLeithner it shouldn't effect, it's an old bug and these PR is also required for the already merged Save2Menu Feature #26681 ! If it couldn't be merged, we could also not go with the merged PR #26681

@HLeithner
Copy link
Member

The shouldn't be any problem that this parameter get's to the model, JTable wouldn't save it if the field doesn't exists.

The only question is, are there any site effects for any component if this exists now? I don't think so but maybe I'm missing something.

@HRIT-Florian
Copy link
Contributor Author

Yes, but we did fixed that, because it is definetly required for #26681 to pass the data from the article. Without that PR, #26681 will definetly not run. At the PBF we had tested it also with other components. Unfortunately I cannot answer myself whether it has an effect on any other components. But it couldn't be a huge problem.

@HLeithner HLeithner merged commit 2960c8f into joomla:4.0-dev Dec 6, 2019
@HLeithner
Copy link
Member

Thanks.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 6, 2019
@HLeithner HLeithner added this to the Joomla 4.0 milestone Dec 6, 2019
@HRIT-Florian
Copy link
Contributor Author

Thanks.

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.

6 participants