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

[5.2] [GSoC 21] Core Enhancement 3 - Merge Featured into Articles #35228

Closed

Conversation

YatharthVyas
Copy link
Contributor

@YatharthVyas YatharthVyas commented Aug 19, 2021

Project Repo PR: joomla-projects/gsoc21_core-enhancements#11
Plan Document: https://docs.google.com/document/d/1Pl8JGa2hkYkmJzQOn9_mS8a4imDmqc2a/edit#heading=h.t5tri5tytv7q


  • Drop-down selector in Articles view
  • Merge Model
  • Merge tmpl files
  • Merge View
  • Merge Controller
  • Update all links pointing to view=featured
  • Update Link of Featured Articles Menu Item
  • Remove Featured files

Summary of Changes

[A] New Selector Dropdown

A searchtools selector is added that can be used to toggle between:

Option Value
All ""
Unfeatured 0
Featured 1

dropdown


[B] Model Merged

  • A new method to return the featured selector filter parameter using the getUserState from request
$featured = $this->getUserStateFromRequest($this->context . '.featured', 'featured', '');
  • The advantage of the above is that we can also pass the value as a GET param to the URL which helps us to make redirect URLs that can initialize the value of this dropdown
  • $featured variable is used as a condition to manipulate the query to adjust to Featured

[C] Templates Merged

  • We use state->get('featured') to conditonally render the template code as per featured or not.

[D] Views Merged

  • We use state->get('featured') to conditonally render the toolbar options

[E] Controller Merged

  • Merged the delete function from FeaturedController

Testing Instructions

  • Visit the Articles View and ensure that everything works as it did before.
  • Test workflows and filters too
  • Ensure that the tmpl and view (and all other MVC files) are common for both featured and articles

Expected result AFTER applying this Pull Request

Everything works normally


Mentors

@chmst @nibra @bembelimen
(and thanks @richard67)

@PhilETaylor

This comment was marked as abuse.

* @package Joomla.Administrator
* @subpackage com_content
*
* @copyright (C) 2019 Open Source Matters, Inc. <https://www.joomla.org>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its a moved file from
administrator/components/com_content/tmpl/featured/default_stage_footer.php

* @package Joomla.Administrator
* @subpackage com_content
*
* @copyright (C) 2019 Open Source Matters, Inc. <https://www.joomla.org>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above its not new just moved

@YatharthVyas
Copy link
Contributor Author

Does this fix #29942 ?

Sadly, No.
I haven't made any change that affects this.

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Aug 20, 2021
@Bakual
Copy link
Contributor

Bakual commented Aug 20, 2021

If you only moved the "Featured" filter to set it using GET, that works as well with: /administrator/index.php?option=com_content&view=articles&filter[featured]=1.

@Quy
Copy link
Contributor

Quy commented Aug 20, 2021

Sorting by Featured does not update the icon-sort to caret up/caret down.

@joomla-cms-bot joomla-cms-bot removed the Language Change This is for Translators label Oct 30, 2021
@YatharthVyas
Copy link
Contributor Author

Updates:

  1. This has been fixed:

Sorting by Featured does not update the icon-sort to caret up/caret down.

  1. All old files related to featured articles have been deleted

@YatharthVyas YatharthVyas marked this pull request as ready for review October 31, 2021 04:55
@Quy
Copy link
Contributor

Quy commented Nov 21, 2021

Will Featured Articles in the sidebar be removed?

@YatharthVyas
Copy link
Contributor Author

Will Featured Articles in the sidebar be removed?

No, it will stay but it's navigation url will be different. This new url will point to the articles (instead of featured) view and it will pass the filter of featured=1 that will be applied in the search tools to only show featured articles

@Quy
Copy link
Contributor

Quy commented Nov 21, 2021

Two menu items are highlighted. Also, see #28003 for request to remove it.

featured-articles

@brianteeman
Copy link
Contributor

The double highlighting is because of an unrelated bug in menu highlighting

@bembelimen bembelimen added this to the Joomla 4.1 milestone Nov 30, 2021
@bembelimen bembelimen modified the milestones: Joomla 4.1, Joomla 4.2 Jan 7, 2022
@chmst
Copy link
Contributor

chmst commented Jan 22, 2022

#19886 could be obsolete for 4.x with this PR


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

@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.0-dev. No new features will be merged into Joomla! 4.3 series. Joomla! 4.4 series is a bridge release to make migration from Joomla! 4 to 5 as smooth as possible.

@HLeithner HLeithner changed the base branch from 5.0-dev to 5.1-dev September 30, 2023 22:52
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev April 24, 2024 09:10
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@HLeithner HLeithner changed the title [4.3] [GSoC 21] Core Enhancement 3 - Merge Featured into Articles [5.2] [GSoC 21] Core Enhancement 3 - Merge Featured into Articles Apr 24, 2024
@ChrisHoefliger
Copy link

I have tested this item 🔴 unsuccessfully on 293c6b9

On applying this I get the following error as soon as I open the list of articles:

500 Layout default_batch_footer not found.


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

@richard67
Copy link
Member

On applying this I get the following error as soon as I open the list of articles:

500 Layout default_batch_footer not found.

@ChrisHoefliger Please check each pull request (PR) on GitHub before testing. If the PR shows conflicting files at the bottom on GitHub, it doesn't make much sense to test it. Please change back your test result to "Not tested". Thanks in advance.

@crimle
Copy link

crimle commented Jul 15, 2024

I have tested this item 🔴 unsuccessfully on 293c6b9

Before applying teh patch, astonishingly the feature was already there.
After applying the patch, I am getting this error:
An error has occurred.
500 Layout default_batch_footer not found.


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

@ChrisHoefliger
Copy link

I have not tested this item.


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

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on 293c6b9Before applying teh patch, astonishingly the feature was already there. After applying the patch, I am getting this error: An error has occurred. 500 Layout default_batch_footer not found.

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

You must have misunderstood as it is not already present

@brianteeman
Copy link
Contributor

Thank you @brianteeman for pointing this out. We are going to contact student and mentor about this enhancement as it was a GSoC project. We will also double-check with the UX team on this one.

??

@cybersalt
Copy link

I have tested this item 🔴 unsuccessfully on 293c6b9

I tested this and got a 500 error. Here is the error stack:

An error has occurred.
500 Layout default_batch_footer not found.
Call Stack

Function Location

1 () JROOT/libraries/src/MVC/View/HtmlView.php:425
2 Joomla\CMS\MVC\View\HtmlView->loadTemplate() JROOT/administrator/components/com_content/tmpl/articles/default.php:416
3 include() JROOT/libraries/src/MVC/View/HtmlView.php:416
4 Joomla\CMS\MVC\View\HtmlView->loadTemplate() JROOT/libraries/src/MVC/View/HtmlView.php:204
5 Joomla\CMS\MVC\View\HtmlView->display() JROOT/administrator/components/com_content/src/View/Articles/HtmlView.php:151
6 Joomla\Component\Content\Administrator\View\Articles\HtmlView->display() JROOT/libraries/src/MVC/Controller/BaseController.php:697
7 Joomla\CMS\MVC\Controller\BaseController->display() JROOT/administrator/components/com_content/src/Controller/DisplayController.php:65
8 Joomla\Component\Content\Administrator\Controller\DisplayController->display() JROOT/libraries/src/MVC/Controller/BaseController.php:730
9 Joomla\CMS\MVC\Controller\BaseController->execute() JROOT/libraries/src/Dispatcher/ComponentDispatcher.php:143
10 Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch() JROOT/libraries/src/Component/ComponentHelper.php:361
11 Joomla\CMS\Component\ComponentHelper::renderComponent() JROOT/libraries/src/Application/AdministratorApplication.php:150
12 Joomla\CMS\Application\AdministratorApplication->dispatch() JROOT/libraries/src/Application/AdministratorApplication.php:195
13 Joomla\CMS\Application\AdministratorApplication->doExecute() JROOT/libraries/src/Application/CMSApplication.php:306
14 Joomla\CMS\Application\CMSApplication->execute() JROOT/administrator/includes/app.php:58
15 require_once() JROOT/administrator/index.php:32


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

@richard67
Copy link
Member

richard67 commented Aug 20, 2024

I have tested this item 🔴 unsuccessfully on 293c6b9I tested this and got a 500 error. Here is the error stack:

@cybersalt I would say that's expected. If you go to this PR here on GitHub #35228 and scroll to the bottom you will see that this PR has lots of conflicting files. As long as this is the case, it doesn't make sense to test this PR.

See also my previous comment here #35228 (comment) .

@chmst
Copy link
Contributor

chmst commented Aug 20, 2024

This PR is so good and should go into 6.0 due to b/c breaks it cannot go into 5.x.
Meanwhile there were so many changes also in code styles, that a new PR is open. #43907.
The new one is based completely on this one - all descriptions and instructions are still valid.

Closing this one for now to avoid confusion.

@chmst chmst closed this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.