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] Frontend Article submit in single language #19302

Merged
merged 4 commits into from
Feb 1, 2018

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Jan 5, 2018

Pr for #19260

I didnt realise that the frontend didnt use the global edit layout so the code to set language to all was not present

Steps to reproduce the issue

  1. Install J4 with Blog Sample Data
  2. Add a Menu Item Type: Create Article to the Main Menu Blog Menu with Access of Registered.
  3. Select Blog as the Category in the Options Tab for that Menu Item.
  4. Go to the frontend and login (as any UG. I did it as a Super User.)
  5. Click the new Menu Item you just created and add a Title and a bit of text if desired.
  6. Save it.

Expected result

That the article would save.

Actual result

Error: Save failed with the following error: Field 'language' doesn't have a default value and the article will not save.

Thanks to @bayareajenn for reporting this

Pr for joomla#19260

I didnt realise that the frontend didnt use the global edit layout so the code to set language to all was not present

## Steps to reproduce the issue
Install J4 with Blog Sample Data
Add a Menu Item Type: Create Article to the Main Menu Blog Menu with Access of Registered.
Select Blog as the Category in the Options Tab for that Menu Item.
Go to the frontend and login (as any UG. I did it as a Super User.)
Click the new Menu Item you just created and add a Title and a bit of text if desired.
Save it.

## Expected result
That the article would save.

## Actual result
Error: Save failed with the following error: Field 'language' doesn't have a default value and the article will not save.
@bayareajenn
Copy link

I have tested this item ✅ successfully on 63e58f3

Saved successfully after applying PR. Thanks so much @brianteeman


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

@infograf768
Copy link
Member

Looks fine here.

@ghost
Copy link

ghost commented Jan 6, 2018

@infograf768 successfully test?

@astridx
Copy link
Contributor

astridx commented Jan 9, 2018

I have tested this item ✅ successfully on 63e58f3


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

@astridx
Copy link
Contributor

astridx commented Jan 9, 2018

I have tested this item successfully
I installed J4 with Blog Sample Data and added a Menu Item Type: Create Article to the Main Menu Blog Menu with Access of Registered. I selected Blog as the Category in the Options Tab for that Menu Item.
I logged in as Super User and created a simple article.

I got the error message without the patch and after that the article was successful saved.screen shot 2018-01-09 at 00 41 12
screen shot 2018-01-09 at 00 41 22


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

@Quy
Copy link
Contributor

Quy commented Jan 9, 2018

RTC

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 9, 2018
@brianteeman brianteeman added this to the Joomla 3.8.4 milestone Jan 9, 2018
@Quy
Copy link
Contributor

Quy commented Jan 9, 2018

@brianteeman This should be tagged for v4.0.

@brianteeman brianteeman modified the milestones: Joomla 3.8.4, Joomla 4.0 Jan 9, 2018
@brianteeman
Copy link
Contributor Author

Sorry I was on my phone. Corrected the milestone now

@bembelimen
Copy link
Contributor

I think we should not add this hidden field hardcode, but we should change the field type...

@brianteeman
Copy link
Contributor Author

Why

@bembelimen
Copy link
Contributor

If it's an fields, it's e.g. changeable via a plugin.

@brianteeman
Copy link
Contributor Author

I think you have completely misunderstood this pr

@bembelimen
Copy link
Contributor

bembelimen commented Jan 16, 2018

Eh no?

If someone has an extension for their own multilingual stuff, he/she/it has to override this line instead of creating a plugin which changes the behavior.

@brianteeman
Copy link
Contributor Author

@bembelimen that would be beyond the scope of this pr as they would have to change other things as well and not just this field. This pr is just fixing a bug in my original PR. (I still dont see the need for the change you requested as all they have to do is to enable the core language filter plugin and then they dont have to do anything)

@bembelimen
Copy link
Contributor

I sent you a PR which:

  1. uses a FormField to displaying the language field
  2. Make sure, that a user cannot manipulate the value (at the moment a user can insert any language, although it should be *)

https://github.com/brianteeman/joomla-cms/pull/68/files

Anyways, if you think, your's enough, trash my PR

@brianteeman
Copy link
Contributor Author

As i already said - your pr is beyond the scope of this pr AND it doesnt address all uses

@HLeithner
Copy link
Member

@brianteeman the PR is not nice you using a hardcoded input field that's not the Joomla way for formfields. @bembelimen PR does the same but in the correct way.

@brianteeman
Copy link
Contributor Author

As stated already this PR is correcting a bug in a previous larger PR.

If you want to change it to the code proposed by @bembelimen (and i have no issue with it) then it will need to be changed in far more places than just the file touched in this PR so it is out of scope of this PR and the fix proposed here would not be correct as it would not be complete.

@brianteeman
Copy link
Contributor Author

because i simply cant be bothered i will update the pr - i trust that you will test it

@HLeithner
Copy link
Member

@brianteeman thx for the update i test it

@HLeithner
Copy link
Member

I have tested this item ✅ successfully on 1891812


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

@bayareajenn
Copy link

I have tested this item 🔴 unsuccessfully on 1891812

When testing on a clean install (except the file I changed for edit.php earlier in this PR) adding an article from the frontend I get an error of:

0 Using $this when not in object context

Now I very well might be doing something wrong with changing the files. I just copied the full file for FormModel.php and edit.php from the Files changed tab of github and loaded them back to their proper places in the file structure but I cannot get them to fly.

Additionally, I have questions about the changes because of not hardcoding and how you @brianteeman say that it will make a ton of other stuff need to change. That scares me since I don't know what else to test to see what happens with the new file(s) to find whatever other issues will come of this.

I know it's late for all of you. I couldn't get to this any earlier today and I'm always 8-9 hours behind you guys no matter what I do.

Anyways, I didn't get far testing this PR. :(


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

@brianteeman
Copy link
Contributor Author

@bayareajenn always use the patchtester component when testing and not manually editing files

@bayareajenn
Copy link

Ok. I'll do it over with patchtester.


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

@bayareajenn
Copy link

I can't even get patchtester to do anything in J4. I'll ask Niels to help me with it. Might be tomorrow that he gets back to me though.


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

@brianteeman
Copy link
Contributor Author

Just hit refresh :)

My comment about other places is probably inaccurate. It was my mistake as everywhere else uses a layout and that's ok

@bayareajenn
Copy link

@brianteeman Oh good. I am happy that it won't create issues anywhere else. Thanks.

Yes, I've refreshed. I can get nothing at all to come up in patchtester. It's ok though. It's an opportunity for me to learn more and that is never a bad thing.

@astridx
Copy link
Contributor

astridx commented Jan 16, 2018

I have tested this item ✅ successfully on 1891812


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

@bayareajenn
Copy link

I have tested this item ✅ successfully on 1891812

Worked great when I used Patch Tester. I must have messed up yesterday. Niels helped me.


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

@brianteeman
Copy link
Contributor Author

@bayareajenn great and thanks

@wilsonge wilsonge merged commit 985bd97 into joomla:4.0-dev Feb 1, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 1, 2018
@wilsonge
Copy link
Contributor

wilsonge commented Feb 1, 2018

Thanks guys!

@brianteeman brianteeman deleted the frontend_edit branch February 2, 2018 01:48
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.

None yet

9 participants