Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Ticket 91 #92

Merged
merged 8 commits into from

4 participants

@nmpetkov

Improving caching in News module: Ticket #91

@ghost Unknown referenced this pull request in zikula/core
Merged

Optimize Theme caching: Ticket #315 #317

@hvorragend
Owner

@espaan

Do we need two branches (master+release-3.0.x) at the moment? As far as I know there is the same code in both branches. I deleted the release-branch in EZComments, too.

So it is easier to merge the pull request into both branches.

We can create a new topic branch later on.

@espaan
Owner
@hvorragend hvorragend merged commit 34e856c into zikula-modules:release-3.0.x
@craigh
Owner

+1 to removing additional branches. I have done the same in other projects as well. It sounds like Erik is too busy ATM, so Carsten feel free to do it IMO. erik?

@craigh
Owner

@drak is there a way to ensure both branches are identical?

@ghost

Removing branches is a big mistake.

@craigh
Owner

drak - is there any way to ensure branches are identical?

WHY do you believe removing branches is a bad idea?

@ghost

@craigh - you can just do git diff master release-3.0.x to see if they are different. If the master branch and release-3.0.x branch are truly identical then you could of course just delete the release-3.0.x branch but then you are varying the workflow of bugfix ad feature branches. You are definitely going to need the master branch soon for Core 1.4.

There is also a misconception that you must merge immediately up all branches. It is common in large projects to merge up in chunks, for the case of News, if there really is no work going on in master, then you could just leave it, and once work begins it's just one command, git merge release-3.0.x after checking out master. In core, for example, I tend to merge into master every few days, not immediately for every PR on release-1.3.

@nmpetkov

I think some projects are suitable, some not (to have branches). If development of a project keeps always in working (production) shape, branches are for historical purposes and some bug removal in old versions.
If a project will be refactored in "dangerous" way - keep "always" working ("production") branch, but master is for those big changes.
For example AddressBook, which I found is no working at all in current project, lacks of working ("legacy"?) branch. And to have such a working branch I opened one more project :-)
Actually suitable place for this working project I to be branch for main project.

It is also good idea to make note at project description: what to download for usage, and where to make pull requests for development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
3  src/modules/News/lib/News/Api/User.php
@@ -511,8 +511,9 @@ public function getArticlePreformat($args)
}
// Allowed to read full article?
+ // DataUtil::formatForDisplay($info['title']) resolves display bug if double quotes exist in title
if (SecurityUtil::checkPermission('News::', "{$info['cr_uid']}::{$info['sid']}", ACCESS_READ)) {
- $title = '<a href="'.$links['fullarticle'].'" title="'.$info['title'].'">'.$info['title'].'</a>';
+ $title = '<a href="'.$links['fullarticle'].'" title="'.DataUtil::formatForDisplay($info['title']).'">'.$info['title'].'</a>';
$print = '<a class="news_printlink" href="'.$links['print'].'">'.$this->__('Print').' <img src="images/icons/extrasmall/printer.png" height="16" width="16" alt="[P]" title="'.$this->__('Printer-friendly page').'" /></a>';
$printicon = '<a class="news_printlink" href="'.$links['print'].'"><img src="images/icons/extrasmall/printer.png" height="16" width="16" alt="[P]" title="'.$this->__('Printer-friendly page').'" /></a>';
} else {
View
3  src/modules/News/lib/News/Controller/Admin.php
@@ -383,6 +383,9 @@ public function delete($args)
// Let any hooks know that we have deleted an item
$this->notifyHooks(new Zikula_ProcessHook('news.ui_hooks.articles.process_delete', $sid));
+
+ // clear article and view caches
+ News_Controller_User::clearArticleCaches($item, $this);
}
return $this->redirect(ModUtil::url('News', 'admin', 'view'));
View
75 src/modules/News/lib/News/Controller/User.php
@@ -310,6 +310,7 @@ public function create($args)
*
* @author Mark West
* @param 'page' starting number for paged view
+ * @param $args['giventemplate'] - Template file to use
* @return string HTML string
*/
public function view($args = array())
@@ -323,7 +324,6 @@ public function view($args = array())
$modvars = $this->getVars();
// Get parameters from whatever input we need
- $theme = isset($args['theme']) ? strtolower($args['theme']) : (string)strtolower(FormUtil::getPassedValue('theme', 'user', 'GET'));
$page = isset($args['page']) ? $args['page'] : (int)FormUtil::getPassedValue('page', 1, 'GET');
$prop = isset($args['prop']) ? $args['prop'] : (string)FormUtil::getPassedValue('prop', null, 'GET');
$cat = isset($args['cat']) ? $args['cat'] : (string)FormUtil::getPassedValue('cat', null, 'GET');
@@ -331,9 +331,7 @@ public function view($args = array())
$defaultItemsPerPage = ($displayModule == 'X') ? $modvars['storyhome'] : $modvars['itemsperpage'];
$itemsperpage = isset($args['itemsperpage']) ? $args['itemsperpage'] : (int)FormUtil::getPassedValue('itemsperpage', $defaultItemsPerPage, 'GET');
$displayonindex = isset($args['displayonindex']) ? (int)$args['displayonindex'] : FormUtil::getPassedValue('displayonindex', null, 'GET');
-
- $allowedThemes = array('user', 'rss', 'atom', 'printer');
- $theme = in_array($theme, $allowedThemes) ? $theme : 'user';
+ $giventemplate = isset($args['giventemplate']) ? $args['giventemplate'] : 'view.tpl';
// work out page size from page number
$startnum = (($page - 1) * $itemsperpage) + 1;
@@ -395,6 +393,12 @@ public function view($args = array())
// assign the root category
$this->view->assign('category', $cat);
$this->view->assign('catname', isset($catname) ? $catname : '');
+ $this->view->assign('catimagepath', $this->getVar('catimagepath'));
+
+ $accesslevel = ACCESS_READ;
+ if (SecurityUtil::checkPermission('News::', "::", ACCESS_COMMENT)) $accesslevel = ACCESS_COMMENT;
+ if (SecurityUtil::checkPermission('News::', "::", ACCESS_EDIT)) $accesslevel = ACCESS_EDIT;
+ $accesslevel = '|a'.$accesslevel;
$newsitems = array();
// Loop through each item and display it
@@ -403,7 +407,7 @@ public function view($args = array())
if (($item['published_status'] == 0) &&
(!isset($displayonindex) || $item['displayonindex'] == $displayonindex)) {
- $template = $theme . '/index.tpl';
+ $template = 'user/index.tpl';
if (!$this->view->is_cached($template, $item['sid'])) {
// $info is array holding raw information.
// Used below and also passed to the theme - jgm
@@ -426,7 +430,7 @@ public function view($args = array())
'preformat' => $preformat));
}
- $newsitems[] = $this->view->fetch($template, $item['sid']);
+ $newsitems[] = $this->view->fetch($template, $item['sid'].$accesslevel);
}
}
@@ -446,7 +450,7 @@ public function view($args = array())
'itemsperpage' => $itemsperpage));
// Return the output that has been generated by this function
- return $this->view->fetch($theme . '/view.tpl');
+ return $this->view->fetch('user/'.$giventemplate);
}
/**
@@ -472,10 +476,6 @@ public function display($args)
// User functions of this type can be called by other modules
extract($args);
- $theme = isset($args['theme']) ? strtolower($args['theme']) : (string)strtolower(FormUtil::getPassedValue('theme', 'user', 'GET'));
- $allowedThemes = array('user', 'printer');
- $theme = in_array($theme, $allowedThemes) ? $theme : 'user';
-
// At this stage we check to see if we have been passed $objectid, the
// generic item identifier
if ($objectid) {
@@ -504,13 +504,6 @@ public function display($args)
}
}
- // For caching reasons you must pass a cache ID
- if (isset($sid)) {
- $this->view->setCacheId($sid . $page);
- } else {
- $this->view->setCacheId($title . $page);
- }
-
// Get the news story
if (!SecurityUtil::checkPermission('News::', "::", ACCESS_ADD)) {
if (isset($sid)) {
@@ -544,6 +537,13 @@ public function display($args)
return LogUtil::registerError($this->__('Error! No such article found.'), 404);
}
+ // For caching reasons you must pass a cache ID
+ $accesslevel = ACCESS_READ;
+ if (SecurityUtil::checkPermission('News::', "::", ACCESS_COMMENT)) $accesslevel = ACCESS_COMMENT;
+ if (SecurityUtil::checkPermission('News::', "::", ACCESS_EDIT)) $accesslevel = ACCESS_EDIT;
+ $pagedir = ($page>1) ? '|pg'. $page : '';
+ $this->view->setCacheId($sid .'|a'. $accesslevel . $pagedir);
+
// Explode the story into an array of seperate pages
$allpages = explode('<!--pagebreak-->', $item['bodytext']);
@@ -587,6 +587,7 @@ public function display($args)
$modvars = $this->getVars();
$this->view->assign('lang', ZLanguage::getLanguageCode());
+ $this->view->assign('catimagepath', $this->getVar('catimagepath'));
// get more articletitles in the categories of this article
if ($modvars['enablecategorization'] && $modvars['enablemorearticlesincat']) {
@@ -622,7 +623,7 @@ public function display($args)
'itemsperpage' => 1));
// Return the output that has been generated by this function
- return $this->view->fetch($theme . '/article.tpl');
+ return $this->view->fetch('user/article.tpl');
}
/**
@@ -656,7 +657,8 @@ public function archives($args)
$monthnames = explode(' ', $this->__('January February March April May June July August September October November December'));
// Create output object
- $cacheid = "$month|$year";
+ // For caching reasons you must pass a cache ID
+ $cacheid = '_'.$year.'|'.$month; // to prevent colision year with article id
$this->view->setCacheId($cacheid);
$template = 'user/archives.tpl';
@@ -1075,19 +1077,26 @@ public static function clearArticleCaches($item, $controller)
{
// clear appropriate caches
// view.tpl templates are not cached
- $pagecount = count(explode('<!--pagebreak-->', $item['bodytext']));
- if ($pagecount < 1) {
- $pagecount = 1;
- }
- for ($i = 1; $i <= $pagecount; $i++) {
- $cacheid = $item['sid'] . $i;
- $cacheid_title = $item['title'] . $i;
- $controller->view->clear_cache('user/article.tpl', $cacheid);
- $controller->view->clear_cache('user/article.tpl', $cacheid_title);
- $controller->view->clear_cache('printer/article.tpl', $cacheid);
- $controller->view->clear_cache('printer/article.tpl', $cacheid_title);
- $controller->view->clear_cache('user/articlepdf.tpl', $item['sid']); // pdf only uses sid
- $controller->view->clear_cache('user/articlepdf.tpl', $item['title']); // pdf only uses title
+
+ // Clear View_cache (module themplate cache) for given article Id
+ $cacheid = $item['sid'];
+ $controller->view->clear_cache(null, $cacheid); // npetkov added !
+ // clear Theme_cache, if any
+ $cache_ids = array();
+ $cache_ids[] = 'News/user/display/sid_'.$item['sid']; // for given article Id, according to new cache_id structure in Zikula 1.3.2.dev (1.3.3)
+ $cache_ids[] = 'homepage'; // for homepage (it can be adjustment in module settings)
+ $cache_ids[] = 'News/user/view'; // view function (articles list)
+ $cache_ids[] = 'News/user/main'; // main function
+ $cache_ids[] = 'News/user/archives/month_'.DateUtil::getDatetime_Field($item['ffrom'], 2).'/year_'.DateUtil::getDatetime_Field($item['ffrom'], 1); // archives
+ $theme = Zikula_View_Theme::getInstance();
+ //if (Zikula_Core::VERSION_NUM > '1.3.2') {
+ if (method_exists($theme, 'clear_cacheid_allthemes')) {
+ $theme->clear_cacheid_allthemes($cache_ids);
+ } else {
+ // clear cache for current theme only
+ foreach ($cache_ids as $cache_id) {
+ $theme->clear_cache(null, $cache_id);
+ }
}
}
Something went wrong with that request. Please try again.