User tests: Successful: Unsuccessful:
Archived articles view year selection is set to 2002 instead of going from current year to 10 years before.
Labels |
Added:
?
|
It serves as a placeholder, although I should have made it 2000 instead. I guess I was not thinking correctly at the time.
Even 2000 is just an arbitary number.
Imho, if $year -10
doesn't work, we shouldn't hardcode it to a fixed year.
Either it should be based on the dates of the available articles (needs an own db query) or it should be a parameter.
You can't work from the data array as it only contains a subset of the articles (those already filtered by eg limit).
I do something similar in my own component (https://github.com/Bakual/SermonSpeaker/blob/master/com_sermonspeaker/site/models/sermons.php#L388-L415). Going from that I would try something like this:
/**
* Method to get the available years
*
* @return array Array of years
*/
public function getYears()
{
$params = $this->state->params;
$order_date = $params->get('order_date');
$db = $this->getDbo();
$query = $db->getQuery(true);
$query->select('DISTINCT YEAR(`' . $order_date . '`) AS `year`);
$query->from('`#__content`');
$query->where("`$order_date` != '0000-00-00'");
// Filter by archived state
$query->where('state = 2');
$query->order('`year` ASC');
$db->setQuery($query, 0);
$options = $db->loadAssocList();
return $options;
}
I'd put that into the archive model and use the returned list to create the dropdown list.
@tue41582 @Bakual If you like it (and want to test it in detail):
A bit more complex. Fetches only years that are relevant for current user, language and so on, also years of published articles inside archived categories:
Model:
public function getYears()
{
$user = JFactory::getUser();
$params = $this->getState('params');
$db = $this->getDbo();
$query = $db->getQuery(true);
$query->select('a.created');
$query->select('CASE WHEN a.modified = ' . $db->quote($db->getNullDate()) . ' THEN a.created ELSE a.modified END as modified');
$query->select('CASE WHEN a.publish_up = ' . $db->quote($db->getNullDate()) . ' THEN a.created ELSE a.publish_up END as published');
$query->from('#__content AS a');
$query->join('LEFT', '#__categories AS c ON c.id = a.catid');
$query->join('LEFT', '#__categories as parent ON parent.id = c.parent_id');
$subquery = 'SELECT cat.id as id FROM #__categories AS cat JOIN #__categories AS parent ';
$subquery .= 'ON cat.lft BETWEEN parent.lft AND parent.rgt ';
$subquery .= 'WHERE parent.extension = ' . $db->quote('com_content');
$subquery .= ' AND parent.published = 2 GROUP BY cat.id ';
$publishedWhere = 'CASE WHEN badcats.id is null THEN a.state ELSE 2 END';
$query->join('LEFT OUTER', '(' . $subquery . ') AS badcats ON badcats.id = c.id');
if (!$params->get('show_noauth'))
{
$groups = implode(',', $user->getAuthorisedViewLevels());
$query->where('a.access IN (' . $groups . ')')
->where('c.access IN (' . $groups . ')');
}
$query->where($publishedWhere . ' = 2');
$nullDate = $db->quote($db->getNullDate());
$nowDate = $db->quote(JFactory::getDate()->toSql());
if ((!$user->authorise('core.edit.state', 'com_content')) && (!$user->authorise('core.edit', 'com_content')))
{
$query->where('(a.publish_up = ' . $nullDate . ' OR a.publish_up <= ' . $nowDate . ')')
->where('(a.publish_down = ' . $nullDate . ' OR a.publish_down >= ' . $nowDate . ')');
}
if (JLanguageMultilang::isEnabled())
{
$query->where('a.language IN (' . $db->quote(JFactory::getLanguage()->getTag()) . ',' . $db->quote('*') . ')');
}
$db->setQuery($query);
$years = $db->loadObjectList();
$order_date = $params->get('order_date', 'published');
$years = JArrayHelper::getColumn($years, $order_date);
JArrayHelper::toInteger($years);
$years = array_unique($years);
sort($years);
return $years;
}
view.html.php
// Year Field
$years = array();
$years[] = JHtml::_('select.option', null, JText::_('JYEAR'));
$Years = $this->get('Years');
foreach ($Years as $Year)
{
$years[] = JHtml::_('select.option', $Year, $Year);
}
I wouldn't make it that complicate. We don't want to duplicate the whole articles query. We need something simple and fast.
It doesn't matter if there is a year without a valid article. It will just return an empty list then.
In fact it would probably be sufficient to just fetch the smallest year in the database and just build each year going from there to present.
Suggest to change it to a layout, that way people can at least override it when needed! You can use the query from the module.
I wouldn't make it that complicate. We don't want to duplicate the whole articles query. We need something simple and fast
It's not the whole query but I agree.
But on the other hand if a category is archived and articles in it are published, the simple way doesn't display these years. So, makes absolutely no sense because filter options are incomplete.
A simple min/max query also makes no sense without joining over categories because of the same reason. One could remove language and access parts but then you'll get empty years in filter.
You can use the query from the module.
No, because
You can't work from the data array as it only contains a subset of the articles (those already filtered by eg limit).
but then you'll get empty years in filter
Which is no issue at all and we have that currently as well if you don't have archived articles then years back.
Ofcourse you need to take off the limit or at least change it to a limit appropriate for this functionality.
Users do have issues with the current way of working, which is why I suggested a layout. People using it as intended can leave it as is. People that don't want the empty years or that only want the archives as produced by the module can provide an override.
It shouldn't change current functionality but introduce the possibility to provide the desired functionality by an override.
I don't think we need a specific JLayout for that single field.
You can override it fine already. You just need to override the view layout (https://github.com/tue41582/joomla-cms/blob/archive/components/com_content/views/archive/tmpl/default.php) and create your own year filter field there. It's not a big or complex layout.
It shouldn't be a JLayout for a single field. It should be for the filter. Also, the module and view should get on the same line. It is ridiculous that the module works on create date and the view on published date! That means you end up on an empty page when they are different!
Disagree on putting processing in the tmpl because you can't override the view.html.php. That is what JLayout is solving!
The menu item (view) lets you choose on which date it should filter. If you also use the module, it sure would make sense to set the menu item to use the created field as well.
The module is hardcoded currently to the created field. Changing that would be another PR.
The module is a different filter thing anyway, there you specify how many months (links) it should show.
Disagree on putting processing in the tmpl because you can't override the view.html.php. That is what JLayout is solving!
You don't need to override the view. You just have to create the HTML needed for a simple select. You can hardcode that very easy in your custom override without any trouble.
It shouldn't be a JLayout for a single field. It should be for the filter.
Did you have a look at the view layout? There isn't much more in it than the filters...
When also using the module, make sure that the menu item is set to use created. Otherwise those links may produce empty lists. Imagine the surprise of a visitor clicking on an archive and being told there are no articles. The number of links produced by the module are irrelevant.
Would consider editing the HTML Layout bad advice, depending ofcourse on what filter you would want. Having the view produce one thing and ignoring that and replacing it with what you need is not the intended use! Accessing the database to get the info you need to produce the html you want in a template is not advisable either.
Yes, I had a look at the view layout. Consider it very bad, it doesn't fit the concept properly! It should be rewritten using JLayouts to allow for proper overriding. And the model should produce the archives.
Which brings us back to the first question: What issue are we addressing here? Changing the year doesn't make sense at all. Maybe ten years of archives doesn't make sense to everybody. What is the motivation to change this?
Accessing the database to get the info you need to produce the html you want in a template is not advisable either.
Personally, if I had to create an override for my own use, I sure wouldn't query the database to get the oldest article. I would just hardcode it. It's very unlikely that I will add an older article in future
Category | ⇒ | Components Layout |
i think this was fixed by #10059
@brianteeman can you check?
Yes it has been
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-04-27 22:45:14 |
Closed_By | ⇒ | brianteeman |
Why hardcoding it to 2002 and not another year?