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
Options, "Show Unauthorised Links" not working as expected #21407
Comments
Please check your template's layout override, or test this against one of the core templates (Protostar uses the default markup and Beez3 has a layout override). Many of these checks are performed in the layouts so if they aren't implemented correctly there then the layout must be updated. |
I checked the override in gantry4 and could not find any difference in logic regarding the readmore button. Furthermore I checked Protostar and experienced the same issue as described in this bug report. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21407. |
I just made a quick test with 3.8.12-dev, PHP 7.2, Template Protostar. I can confirm that setting "Use article settings" works wrong with these settings: Introtext and "Register to read more.." are displayed but shouldn't. |
@infograf768 Maybe related somehow(?) #19406 |
#19406 Test were done when the Featured Menu Item Show Unauthorised Links is set to Yes. Global is set to No
Case A: If the article Therefore we have to solve the 2 last cases, i.e. the article settings are NOT taken into account when the Featured Menu item settings is set to 'Use Article Settings` Have not tested, but it may be the same issue when using blog. |
~~~Will look at it now~~~
I am afraid no.
When doing https://github.com/joomla/joomla-cms/pull/19406 I basically copied the code in category blog and I confirm we have the same bug there. |
The only possibility I see is to remove the setting Let's say we set
One cannot remove articles later in the view that evaluates |
I never used that setting myself. So will not comment on the expected bevahior or what would be wise to do there. |
Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/21407 |
closed as having Pull Request #21450 |
Set to "open" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/21407 |
reopened for further Discussion. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21407. |
It's not that we don't understand the problem theoretically but the practice is showing that it's not possible at the current state of code (= Joomla) to solve it. And I don't think that it's possible in the future
Many extensions (e.g. modules) are using the same code base to fetch articles (= so-called articles model). Anybody can provide a better solution. |
Could you please point me to the relevant developer documentation? This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21407. |
Based on code history, 'Use Article Setting' option was added to menu items by mistake 014b52f#diff-6ffd3a7d782585864181873aea4c2c1fR306. |
Yep, we should have tested more. That one was useless and passed with the others. |
Could someone elaborate that more, that I can understand the technical issue here? I would like to search for a solution too.
|
I myself think that Joomla GitHub is the wrong place to explain the basic MVC design of Joomla (Model-View-Controller). And common Joomla component design. The main thing that I'm talking about starts here: https://github.com/joomla/joomla-cms/blob/3.8.11/components/com_content/models/featured.php#L116 which calls the getItem method from the parent articles model. https://github.com/joomla/joomla-cms/blob/3.8.11/components/com_content/models/articles.php#L557 which uses the filter settings for the database request. The filter settings have been defined earlier depending on the global and/or current menu item settings.
Because it's a bug by mistake ;-) if filter filter.access for the database query is false the $items collection contains ALL registered articles, too. There is no trivial way to manipulate the database query at this point that it also respects the setting for show'_noauth for a single article (JOIN or something). This collection doesn't contain ALL articles of the database. Just the number that is needed for the frontend page. Let's say 20 And let's say that we now check the setting of param show_noauth for any single article in the collection and you remove an article from the collection. Afterwards you have 19 articles. Maybe OK for a special case. BUT: You will open a can of worms (e.g. wrong pagination) for many other cases. |
I better paste the comment that I posted in [#27856] - Category blog Access, since it seams to be a related issue. As I've mentioned in the thread at the Joomla Forum, the "Use Article Setting" in the Menu settings doesn't make sense from a logic point of view. I found the following comment that might explain why. Based on code history, 'Use Article Setting' option was added to menu items by mistake 014b52f#diff-6ffd3a7d782585864181873aea4c2c1fR306. From a logical point of view it would make more sens that the the setting at the Global level is inherited to the Menu level and the settings at the Menu level is inherited to the Article level, then the "Use Article Setting" could be omitted. Since the intro of articles, which are restricted to registered users, are shown even when the setting at the Article level of "Show Unauthorised Links" is set to No, which shouldn't be the case. This make me think that the problem might be the SQL query. Unfortunately, I don't know how the system is built, but it would be of interest to see how the SQL query looks. |
Been made aware of this ticket by @Webdongle As described by @ReLater, a fix for this issue (and I tend to agree that it actually is an issue because it does not match a users expectation) isn't a straightforward thing. The articles are retrieved using one massive SQL query; filtering this last based upon a parameter of an article (that might be set to "use global" which makes stuff even worse) is complex and has a big performance impact. |
@SniperSister |
After taking a closer look at the issue, I tend to agree with @ReLater that the best option is to remove the use_article option in the parameter. @joomla/security any other ideas? |
mmmmmmmmm Remove a feature (that used to work) because it stopped working. Is that not going backwards? Why not change the code (that makes the action) when 'Use Article settings' is selected? |
It would be interesting to see what the massive SQL query looks like :) "Use Article settings" is not logical and I believe it's part of the problem. "Show Unauthorised Links" seams to work down to the Menu level. It's the selection at the Article level that doesn't work. Without knowing how the code is written my logical solution would be:
I can't see the this could complicate things too much since the only thing that is needed to be added is a condition where an article is not going to be shown if "Show Unauthorised Links" at the Article level is set to "No". With this simple solution one should also remove the Yes alternative in the "Show Unauthorised Links" at the Article level, because to be able to show articles they need to inherit Yes from the Menu level to be able to be shown. It would look like the following: Global - Articles - "Show Unauthorised Links": Menu - Options - "Show Unauthorised Links": Article - Options - "Show Unauthorised Links": |
'use article settings' is logical because (with the exception of meta data) menu settings override Article settings.
So for consistency 'show unauthorised' (in category blog menu) should override the Article setting.
Therefore 'use article settings' is logical and necessary. Otherwise individual control of Articles showing/hiding intro text would not be possible.
Sent from BlueMail
…On 10 Feb 2020, 23:21, at 23:21, phpwebtech ***@***.***> wrote:
> Been made aware of this ticket by @Webdongle
>
> As described by @ReLater, a fix for this issue (and I tend to agree
that it actually is an issue because it does not match a users
expectation) isn't a straightforward thing. The articles are retrieved
using one massive SQL query; filtering this last based upon a parameter
of an article (that might be set to "use global" which makes stuff even
worse) is complex and has a big performance impact.
It would be interesting to see what the massive SQL query looks like :)
"Use Article settings" is not logical and I believe it's part of the
problem.
"Show Unauthorised Links" seams to work down to the Menu level. It's
the selection at the Article level that doesn't work.
Without knowing how the code is written my logical solution would be:
- Remove the "Use Article settings" at the Menu level
- Change "Use Global (Yes/No)" at the Article level to "Use Menu
(Yes/No) - The Global setting is inherited to the Menu setting and it
would make sens if the Menu setting is inherited to the Article setting
- Ad the missing selection at the Article level in the SQL query
I can't see the this could complicate things too much since the only
thing that is needed to be added is condition where an article is not
going to be shown if "Show Unauthorised Links" at the Article level is
set to "No".
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#21407 (comment)
|
I don't agree that "Use Article Settings" at the Menu level is logical. Especial not from the user's (administrator) point of view. As an administrator I'm used to set the Global settings, which then is inherited down to the final level (Article level). Even if the setting at the Article level should work, then it's completely illogical that one has to go to the Menu level and change the setting to "Use Article Settings" for the setting at the Article level to work. It would make more sense to have to choose the setting at the Article level instead and it could then look like the following: Global - Articles - "Show Unauthorised Links": Menu - Options - "Show Unauthorised Links": Article - Options - "Show Unauthorised Links": Another alternative is just delete the "Show Unauthorised Links" from the Menu level. Then one can only choose to set the Global and then one has to change every single article at the Article level if one wants a different behaviour then one has chosen as Global. It would then look like the following: Global - Articles - "Show Unauthorised Links": Article - Options - "Show Unauthorised Links": I believe it might have functioned like the last alternative when one is looking at the code.
The value for use "Global" is an empty string It would be good if the code could start to work accordingly to my last alternative, which I believe might have been the case before, so one can change the behaviour of each individual article that one doesn't want to behave in accordance to the chosen Global setting. If one wants to be able to change the setting at the Menu level too, then maybe one should develop the logic in accordance with the first alternative, but where the value of "Use Menu" shouldn't be a long text string. |
You are basing your conclusions on a false premise. It would make more sense if Global settings were inherited down to Menu then Article but they are not. They are separate settings that are not altered ... they have no hierarchical connection what so ever. . i.e. Changing a menu setting does not change the corresponding article setting. Menu settings do not inherit Global settings the default option is set to use global. That is a significant difference to actually inheriting them!!! Here is the complication. With meta data the Article settings override menu settings but with other options the Article settings are not inherited they are just not used. In other words they are not inherited down to Articles and changing Article settings has no affect. Therefore 'Use Article settings' is needed in Category/Featured blog menu items in order to control the display of Articles individually. That is the logic with the current setup. Further to that ... changing the options (instead of fixing the 'Use Article settings' selection) is a major feature change that would have backward compatibility issues. Not to mention the convention of when feature changes should be made. |
Global settings affect all Articles.
Menu item type 'single article' affects only one Article. Thus Article Option settings are redundant.
Menu item type blog affects several Articles. Thus 'Use Article settings' is needed for individual control of Article display.
Menu Option settings and Article Option settings are set to 'use global' by default. They are NOT inherited from global.
Sent from BlueMail
…On 11 Feb 2020, 23:08, at 23:08, phpwebtech ***@***.***> wrote:
> 'use article settings' is logical because (with the exception of meta
data) menu settings override Article settings. So for consistency 'show
unauthorised' (in category blog menu) should override the Article
setting. Therefore 'use article settings' is logical and necessary.
Otherwise individual control of Articles showing/hiding intro text
would not be possible.
I don't agree that "Use Article Settings" at the Menu level is logical.
Especial not from the user (administrator) point of view. As an
administrator I'm used to set change the Global settings, which then is
inherited down to the final level (Article level). Even if the setting
at the Article level should work, then it's completely illogical that
one has to go to the Menu level and change a the setting to "Use
Article Settings" for the setting at the Article level to work. It
would make more sense to have to choose the setting at the Article
level instead and it could the n look like the following:
Global - Articles - "Show Unauthorised Links":
Yes
No
Menu - Options - "Show Unauthorised Links":
Use Global (Yes/No)
Yes
No
Article - Options - "Show Unauthorised Links":
Use Global (Yes/No)
Use Menu (Yes/No)
Yes
No
Another alternative is just delete the "Show Unauthorised Links" from
the Menu level. Then one can only choose to set the Global and then one
has to change every single article at the Article level if one wants a
different behaviour then one has chosen as Global. It would then look
like the following:
Global - Articles - "Show Unauthorised Links":
Yes
No
Article - Options - "Show Unauthorised Links":
Use Global (Yes/No)
Yes
No
I believe it might have functioned like the last alternative when one
is looking at the code.
`<option value="">JGLOBAL_USE_GLOBAL</option>
<option
value="use_article">COM_CONTENT_FIELD_VALUE_USE_ARTICLE_SETTINGS</option>
<option value="0">JNO</option>
<option value="1">JYES</option>`
The value for use "Global" is an empty string
The value for use "No" is 0 (false)
The value for use "Yes" is 1 (true)
But the value for "Use Article Settings" is a "use_article" which is a
text string that is 6 integers long.
It would be good if the code could start to work accordingly to my last
alternative, which I believe might have been the case before, so one
can change the behaviour of each individual article that one doesn't
want to behave in accordance to the chosen Global setting.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#21407 (comment)
|
I'm afraid you're missing my point. To start with it's a good idea to look at the logic a user (administrator) face. When that is done one can start to think of the logic in programming. The Global setting of "Show Unauthorised Links" can be set to Yes or No. This setting is then passed on down in the hierarchy where on usually can chose: Use Global (Y/N), Yes and No. When choosing "Use Global" at the lower level, than it follows any changes that are being made in the Global setting. When choosing Yes or No, then it always use this setting regardless of changes in the Global setting. With only two levels of the hierarchy, then it's quite simple, but when more levels are added, then it becomes more complicated. From a users perspective it's illogical to have to set the "Use Article Settings" at a higher level for the alternatives of Yes and No at a lower level to be able to function. The Yes and No alternatives at the Article level should not change regardless of changes in the Global settings or in any other mid-hierarchy settings like the Menu setting, neither should one have to change any setting at a higher level for the Yes and No alternative to function. As a user I rather have a two level hierarchy that is logical, then a three level hierarchy that is illogical. Since the three level hierarchy of Global-Menu-Article is not working, then it's advisable to temporary roll-back the introduction of "Use Article Settings" until the current problem has been solved. |
@phpwebtech It is you that is missing the point. It is not an hierarchy. e.g. scenario
Without the 'Use Article settings' that scenario would not be possible. Now do you see where you are wrong or are you just going to ignore my explanation of how the system was designed to work? |
From a users point of view it looks like a hierarchy, where the Global setting can be passed on to lower levels. The problem is that the design of the system doesn't work. When using the "Use Article Settings" at the Menu level, the setting of No do not work. If the design doesn't produce an expected result, then the design might be poor. Based on code history, 'Use Article Setting' option was added to menu items by mistake 014b52f#diff-6ffd3a7d782585864181873aea4c2c1fR306. This is the reason why I begun to focus on the logic from the users point of view, to see if one can come up with a better design of this function. It would therefore be very interesting to know what it looked like before the "Use Article Setting" option was added. Your example scenario could also be achieved without the "Use Article Setting" option in a design where "Show Unauthorised Links" is excluded from the Menu level.
The negative side of this design is that one has to change the setting in every single article that one want to behave different from what is set in the Global setting, but that is something I prefer since one doesn't have to change a setting at the Menu level for making the Yes and No settings at the Article level to work, which I find ilogic.
|
They are not passed down. They are selected with the default selection 'use global'. That is independent selection not inherited.
The system does work it is the code that is broken. The feature did work but something was changed that prevented the code (for the option) taking affect. It is not the design that is poor it is the implementation of the code that is poor.
That is incorrect because it is based on the false premise that it is an hierarchy. They are independent settings. Without 'Use Article settings' in the menu item then the Article setting would be ignored.
Again ... That is incorrect because it is based on the false premise that it is an hierarchy. They are independent settings. Without 'Use Article settings' in the menu item then the Article setting would be ignored. |
With your suggestion remove those settings from blog menu item and set Article settings default selection 'Use Global'. I do see your point and think that would be good except for two things.
The solution is fix the code for 'Use Article setting' (in the drop down box) works. It used to work but now that choice has no affect. But if you want to create a PR to fit your vision then I will test it. |
A general principle in the system is that Global settings are "distributed" down to the "Use Global setting" at lower levels otherwise the Global settings would be quite useless. |
No they are not distributed down. Menu only uses Global if that is selected. Without 'Use Article settings' the Article settings are ignored. The problem is not the system. The problem is that the 'Use Article settings' is using Global not Article settings. But like I said .. if you create a patch I will test it. |
Unfortunately, I've never used the system when the "Use Article Setting" have been working. Neither do I know what it looked like before the "Use Article Setting" was introduced. Without knowing how the code is written and "structured", it's difficult to suggest what changes should be done. Even if it's manly to remove code that goes with "Use Article settings", then for sure it require more changes then correcting a bug. I would be happy if you could direct me to the different locations where the code related to "Use Article settings" can be found, so I more easily can have a look and see what the code looks like.
|
I was talking in general, not in particular about the "Show Unauthorised Links". What I meant was that the chosen setting in Global is distributed down to the Global alternative at a lower level, which can be illustrated in the following way: Alt. 1 xx settings at lower level: Alt. 2 xx settings at lower level: The setting in Global is "distributed" to the alternative "Use Global setting", but when one does use another setting then "Use Global setting", then the Global setting has no impact at all.
|
Those settings would not work. The menu setting would be used. Thus All registered intro text would be seen on the blog page or it would not. There would be no way for admin to allow 'teaser' text on the blog page for just a few Articles. |
It was just a general example showing what I meant with variables "distributed" from Global to lower level.
|
Is #19601 the same issue? |
Steps to reproduce the issue
a. "Show Unauthorised Links = use global" or
b. "Show Unauthorised Links = use article setting"
c. "Show Unauthorised Links = yes"
a. registered users only
b. "Show Unauthorised Links = yes"
c. insert readmore item
a. registered users only
b. "Show Unauthorised Links = use global (off)"
c. insert readmore item
Expected result
2a) intro of featured article FOO is shown
2b) intro of featured article FOO is shown
2c) intro of only featured articles FOO and BAR are shown
Actual result
2a) wether FOO nor BAR intro is shown
2b) wether FOO nor BAR intro is shown
2c) all registered article intros are shown
System information (as much as possible)
mysql = 5.6.19-67.0-log, utf8_general_ci
PHP-Version = 7.0.24
Webserver = Apache/2.4.29
Joomla! 3.8.11 Stable [ Amani ] 31-July-2018 14:00 GMT
Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT
Additional comments
Gantry4 template in use, which has an override of com_content, featured, default.php
The text was updated successfully, but these errors were encountered: