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] mod_popular with disabled hits #34257

Merged
merged 2 commits into from
May 29, 2021
Merged

Conversation

brianteeman
Copy link
Contributor

When hits are disabled for articles (in the article options) it makes no sense to display the Most Read Articles module in the site or admin as the contents will never be updated.

This PR changes the output of the module so that instead of a list of non-updating articles a message is displayed.

Site Module

image

Admin Module

image

When hits are disabled for articles it makes no sense to display the Most Read Articles module in the site or admin as the contents will never be updated.

This PR changes the output of the module so that instead of a list of non-updating articles a message is displayed.
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels May 28, 2021
@chmst
Copy link
Contributor

chmst commented May 28, 2021

The module seems to be very old and there are only few use cases when it makes sense, with or without hit counting. I suggest removing it from the dashboard for new installations. @richard67 what do you think?

@richard67
Copy link
Member

The module seems to be very old and there are only few use cases when it makes sense, with or without hit counting. I suggest removing it from the dashboard for new installations. @richard67 what do you think?

@chmst I think that with hit count it sill makes sense and I would like to have that module on my dashboard because I have it for my homepage, too. But that's just my personal opinion, I have nothing to decide and I don't want to have anything to decide.

@brianteeman
Copy link
Contributor Author

I actually use it a lot - so maybe others so as well - dont really care if its default or not. Either way its beyond the scope of this PR to remove it

@chmst
Copy link
Contributor

chmst commented May 28, 2021

It is not in scope of this PR, just a matter of taste.

@brianteeman
Copy link
Contributor Author

we really have to get out of the habit of out of scope comments. Far too often they completely detract from the actual PR. my 2c

@Abernyte-Git
Copy link

I have tested this item ✅ successfully on 1f63709


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

1 similar comment
@ghost
Copy link

ghost commented May 29, 2021

I have tested this item ✅ successfully on 1f63709


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

@Quy
Copy link
Contributor

Quy commented May 29, 2021

Should the module be displayed on the front end in this case?

@brianteeman
Copy link
Contributor Author

Absolutely yes. Think like a user.

I turn off hits in January. In December I decide to add the module but it doesn't appear anywhere - joomla is broken.
This way it appears and they are informed that they need to re-enable hits

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 29, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone May 29, 2021
@chmst chmst merged commit 3637e7b into joomla:4.0-dev May 29, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 29, 2021
@chmst
Copy link
Contributor

chmst commented May 29, 2021

Thanks!

@brianteeman
Copy link
Contributor Author

Thanks

@brianteeman brianteeman deleted the hits_module branch May 29, 2021 22:16
@brianteeman
Copy link
Contributor Author

@SharkyKZ is it your mission in life to just sit there and put the thumbs down. This is an open repo - if you have a comment to make then please do so.

@SharkyKZ
Copy link
Contributor

With maintainers like these it's pointless.

@richard67
Copy link
Member

Thanks for the flowers.

@brianteeman
Copy link
Contributor Author

I dont believe anyone is a mind reader. If there is something wrong then say it

@PhilETaylor

This comment was marked as abuse.

@brianteeman
Copy link
Contributor Author

thats why you have to speak and not thumbs

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

bembelimen pushed a commit that referenced this pull request Nov 23, 2021
* [4.0] Notification icons (#34226)

* remove method allow parent to run (#34118)

Co-authored-by: Richard Fath <[email protected]>

* [4.0] Changing title to tooltip for template preview (#33292)

* [4.0] Changing title to tooltip fpr template preview

* Update location path in description (#34238)

* Fix patterns field check when field empty and not required (#34124)

* [4.0] Finish transition from CSS classes "label-" to "alert-" for the pre-update check (#34227)

* [4.0] mod_popular with disabled hits (#34257)

* [4.0] mod_popular with disabled hits

When hits are disabled for articles it makes no sense to display the Most Read Articles module in the site or admin as the contents will never be updated.

This PR changes the output of the module so that instead of a list of non-updating articles a message is displayed.

* [4.0] Use MVCFactory to create model (#34092)

* Use MVCFactory to create models

* CS

* Fix row selecting, when module is disabled (#34273)

* Media web service implementation.

* Media web service implementation.

* Fix hard coded adapter name default.

* Fix missing url and tempUrl attributes in single item response.

* Remove useless comments.

* Replace 'PATH' with 'STRING' for cleaning of input parameters.

* Adds required parameter checks and removes global exception handling.

* Adds media web service specific language strings.

* Adds exception handling.

* Adds missing exception handler and fixes wrong path return value after move/rename.

* Use exception message as error title.

* Adds proper FileNotFound exception handling for GET requests.

* Fixes handling of status code.

* Fixes some types, formatting and similar stuff.

* Fixes yet another sloppy typo.

* restore file

* Fix some docs

* Adapter endpoint

* Add plugin to install file

* Add tests

* Update api/components/com_media/src/Helper/MediaHelper.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Helper/MediaHelper.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediumModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/View/Media/JsonapiView.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update libraries/src/Error/JsonApi/SaveExceptionHandler.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Controller/MediaController.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Controller/MediaController.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/AdaptersModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* docs

* Update api/components/com_media/src/View/Media/JsonapiView.php

Co-authored-by: Phil E. Taylor <[email protected]>

* int

* cs

* cs

* more cleanup

* tabs

* restore

* Test all endpoints

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Hardening

* Tests fix for drone

* Update Api.php

* Create directory in test with correct permissions

* Add more tests and remove static helper

* Update tests/Codeception/api/com_media/MediaCest.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update tests/Codeception/_support/Helper/Api.php

Co-authored-by: Phil E. Taylor <[email protected]>

* cs

Co-authored-by: Brian Teeman <[email protected]>
Co-authored-by: Phil E. Taylor <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: infograf768 <[email protected]>
Co-authored-by: Quy <[email protected]>
Co-authored-by: Ruud <[email protected]>
Co-authored-by: Tuan Pham Ngoc <[email protected]>
Co-authored-by: Fedir Zinchuk <[email protected]>
Co-authored-by: Pieter-Jan de Vries <[email protected]>
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.

8 participants