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

[fix] Frontend Filtering by author broken in modal when inserting article via xtd #18445

Merged
merged 1 commit into from
Nov 1, 2017

Conversation

infograf768
Copy link
Member

@infograf768 infograf768 commented Oct 30, 2017

Pull Request for Issue #18442

Summary of Changes

As the modal uses multiple, the filter_articles xml in frontend has also to use that.
Is also added here the possibility to filter by No User as in #18440

Testing Instructions

See #18442

Expected result

No more 500 or PHP Warning. Filtering works OK after patch.

@brianteeman

After patch
screen shot 2017-10-30 at 11 02 22

@ghost
Copy link

ghost commented Oct 31, 2017

I have tested this item ✅ successfully on b81f778


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

1 similar comment
@coolcat-creations
Copy link
Contributor

I have tested this item ✅ successfully on b81f778


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

@joomla-cms-bot joomla-cms-bot removed Language Change This is for Translators PR-staging labels Oct 31, 2017
@ghost
Copy link

ghost commented Oct 31, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 31, 2017
@infograf768 infograf768 added this to the Joomla 3.8.2 milestone Oct 31, 2017
@brianteeman
Copy link
Contributor

As the null value is -select author- shouldnt the new string be "no author"


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

@infograf768
Copy link
Member Author

infograf768 commented Oct 31, 2017

used the same string value as in backend when choosing ‘- No User -‘ in the modal. no need to change imho to be consequent.

@infograf768
Copy link
Member Author

I also think that whoever chose the term ‘user’ understood it is selecting a user as author. the result being a user name or none.

@coolcat-creations
Copy link
Contributor

Means this label shows every article that has no author? When does that happen?

@infograf768
Copy link
Member Author

infograf768 commented Nov 1, 2017

Means this label shows every article that has no author? When does that happen?

When you edit an article in backend you can select a user as author. The modal lets you chose No User.

@brianteeman
in fact, after a good night sleep, I think you are right. For this pr as well as the other one for backend, as we filter by author, I suggest we use « None », i.e. JNONE
I propose we let these 2 pr be merged and then make a specific pr to change the string constant.
What do you think? I also contacted @mbabker concerning this merge.

If you prefer No Author, we would just have to add this new string both in front and backend.

@coolcat-creations
Copy link
Contributor

coolcat-creations commented Nov 1, 2017

I just tested it, and it does not work. If i click at "No user" the article gets reasigned to Superuser. (not related to this PR) So it's not possible to show articles that have no user assigned because there is always an author.

Edit: Correction: If i create a new article and select No User it gets assigned to Super User anyway, just when i open it again and remove the user the autor is gone.

@brianteeman
Copy link
Contributor

@infograf768 i agree with JNONE and a new pr for that

@mbabker mbabker merged commit 5510a98 into joomla:staging Nov 1, 2017
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging and removed RTC This Pull Request is Ready To Commit labels Nov 1, 2017
@infograf768
Copy link
Member Author

@infograf768 i agree with JNONE and a new pr for that

will do as well asanother improvement for the managers

@infograf768
Copy link
Member Author

@coolcat-creations
Chosing No User will only work when editing an article already saved.
nouser

@infograf768 infograf768 deleted the fe_articlextdfilter branch November 1, 2017 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants