design of function qtranxf_excludeUntranslatedPostComments

[qtranslate]

design of function qtranxf_excludeUntranslatedPostComments

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 00:08 GMT

---

On second thought, if you really have a different solution, which is tested and ready to go, submit it with pull request, and we will go from there?

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 03:26 GMT

---

What if we add these two lines at the beginning of the function:

if( !isset($clauses['join']) ) return $clauses;
if( strpos($clauses['join'], $wpdb->posts ) === FALSE) return $clauses;

Did your error messages disappear? Is that good enough for you? If you need to still filter them, then try this:

if( !isset($clauses['join']) || empty($clauses['join']) ){
  $clauses['join'] = "JOIN $wpdb->posts ON $wpdb->posts.ID = $wpdb->comments.comment_post_ID";
}elseif( strpos($clauses['join'], $wpdb->posts) === FALSE ){
  //do not break some more complex JOIN
  return $clauses;
}

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 03:34 GMT

---

Please, also tell me if option "Hide Untranslated Content" makes sense to you at all? I personally do not use it, since I think people should decide themselves whether they want to read untranslated to their language post. They might choose to translate it themselves, but when it is hidden, then they do not know that it existed.

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 08:35 GMT

---

Option "Hide Untranslated Content" perfectly makes sense. I run a couple of sites where i have some posts that are just not relevant for the other languages. plus: it's a seo issue - 'duplicate content'...

regarding the 'people should decide themselves' thing: i run a different approach - people who are logged in can choose which languages they understand in the profile settings and see posts of all languages - the main lang (the one they choose in the browser / the url) always first.

the "Hide Untranslated Content" option is especially effective for 'latest posts' widgets. if your last 3 posts are untranslated you could easily drive people away from your site showing foreign headlines in suggested posts, etc.

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 08:38 GMT

---

Ok, point taken. Other people seem to also want it. What do we do about the code?

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 08:41 GMT

---

i see a big drawback in the current approach of filtering for post_content: it just doesn't tell you anything about the actual language of a certain comment if the corresponding post is translated or not. i have multi-lingual posts, people tend to comment in the language they are reading. so the filter hides comments for untranslated posts but still shows comments in a different language.

so it makes much more sense to add a lang meta to every comment when the comment is saved. the language would be the current user language. very easy. i'll provide code asap.

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 08:55 GMT

---

We want to hide the comments for the untranslated posts, not the untranslated comments. We do not care about the language of comment, it can be anything, we just want consistency, if post is hidden, then all comments to it also hidden regardless to their language.

If we also want to filter comments by specific language, then it would be a new and different option.

Could you answer the question, if the code I cited makes your problem go away? Did you figure out under which condition a query without JOIN to posts table happens?

I would much rather to do optimization later, because I plan to re-design the whole backend to be sane, as it is insanely inefficient now. I do not wish to do it in pieces, just for comments now.

Please, answer the question if the code above makes your site work. Do you need filtering for the case when the problem happens. Would be also good if you tell me finally when and how it happens.

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 10:19 GMT

---

i used the second snippet and it works.

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 10:45 GMT

---

what i found out until now: the get_comments(); function by default doesn't JOIN posts. if you query comment meta it adds INNER JOIN wp_commentmeta and only if you query post related stuff (like post_status, post_type, etc.) it adds JOIN wp_posts.

so a save join string would be
"JOIN $wpdb->posts ON $wpdb->posts.ID = $wpdb->comments.comment_post_ID INNER JOIN $wpdb->commentmeta ON ( $wpdb->comments.comment_ID = $wpdb->commentmeta.comment_id )"

you could test for both cases and only skip if the join string is more complex...

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 14:35 GMT

---

checking the core for 'WP_Comment_Query' (https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/comment.php) led me to another major flaw in the current filter. as you can see in line 430 a CACHED version of the query is returned - ignoring any altered clauses! therefore dynamically altering the clauses leads to wrong returns, i.e. wrong languages displayed. especially if users run object caches. i tested this in my 2015 setup. i had to clear the cache to see the correct comments.

there are 2 safe possible solutions:

1. altering the query vars ('pre_get_comments')
2. filter the 'comments_array'

if it's even worse, the same problem applies to the posts! still checking...

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 15:21 GMT

---

it's true. post queries get also cached. so the 'hide untranslated' option is badly broken because it shows the wrong languages...

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 16:55 GMT

---

Why don't it get cached correctly at first time?

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 16:58 GMT

---

I thought this cache is for one session only, different users do not affect each other, do they?

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 17:05 GMT

---

You had to clear cache, because you were doing testing and changed the query in between the calls on the same session, is that correct? This should not happen at live site, will it?

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 17:24 GMT

---

From the page http://codex.wordpress.org/Class_Reference/WP_Object_Cache:

By default, the object cache is non-persistent. This means that data stored in the cache resides in memory only and only for the duration of the request. Cached data will not be stored persistently across page loads unless you install a persistent caching plugin.

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 17:29 GMT

---

Does it happen in your theme, that this query changes language during the same request? Which theme do you use, btw? If this is what you really need, then yes, we can set language in query_vars['lang'] via do_action_ref_array( 'pre_get_comments', array( &$this ) );. The value query_vars['lang'] will not be used by query, but it will change the cache key.

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 20:33 GMT

---

read the source: it generates a md5 cache_key from the DEFAULT $args. any altering of the clauses happens later and is ignored by the function because the cache key is the same not matter what you do with the clauses later. so it returns the new request from the cache assuming that queryvars not changed == output the same.

with a caching plugin the cache is persistent. that means the next user gets YOUR output. wp generates cache for every different set of $args.

so this clauses altering breaks queries with ANY theme. try it with 2015! (i did.)
you are not going to tell the people they can't use caching plugins to hide untranslated posts, right? ;)

[herrvigg]

Comment by side777
Thursday Feb 12, 2015 at 20:38 GMT

---

i wrote an alternative approach for posts filtering today. i set up an action to preg match the languages in use from the title of the current post in the loop and set a post meta IF the meta is not already there. (for backwards comp)
and i set up an action in save_post to update the meta not matter what.

then i alter the query with 'pre_get_posts'.

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 22:03 GMT

---

md5 cache_key from the DEFAULT $args

I understand, but default arguments uniquely defines what is done later, except us adding a fork by language. If we add query_vars['lang'], the cache key will become different enough for the purpose.

with a caching plugin

Which one do you mean? Anyway, what I described with query_vars['lang'] should solve the problem even with persistent cache.

I am very glad you that you have better ideas and wish to implement them. I will be waiting for your pull request here.

Meanwhile I will put in query_vars['lang'] to be done for the moment, so I can release 3.0 now.

[herrvigg]

Comment by johnclause
Thursday Feb 12, 2015 at 22:05 GMT

---

BTW, you never let me know which theme do you use? I am still curious.

[herrvigg]

Comment by johnclause
Friday Feb 13, 2015 at 09:29 GMT

---

2.9.8.9 has your changes. Persistent cache did not work, because they cut all extra keys from query_vars. Could you, please, test if it is ok for you without persistent cache? We will work on persistent cache later.

[herrvigg]

Comment by side777
Friday Feb 13, 2015 at 09:41 GMT

---

so at least have a look at my working solution.. you get in lots of trouble if you break caching. people tend to not read any warnings and complain in the support forums later. believe me, i know exactly what i'm talking about...

if you insist on using a hacky workaround for your 3.0 that you could use any query var from the default list (in the core code, linked above) - i guess a meta value would do the trick. as mentioned before, this would be very hacky for a major release...

[herrvigg]

Comment by side777
Friday Feb 13, 2015 at 09:42 GMT

---

i have never worked with github - as soon as i find out how pulling a request works and where to do it, i'll send you my solution.

[herrvigg]

Comment by side777
Friday Feb 13, 2015 at 09:44 GMT

---

about my theme: i use a selfmade framework with child themes for the sites under my direct control. other sites i admin use several themes...
for testing i use a clean twenty fifteen setup.

[herrvigg]

Comment by johnclause
Friday Feb 13, 2015 at 18:54 GMT

---

any query var from the default list

I did look into it right after the discovery, but there is no one we can change safely as far as I can tell.

Which caching plugin do you use? I can see a lot of problems with persistent cache, which such plugin must deal with. What if one user submits new comment? It is not going to show up for anyone until cache is invalidated? It can get extremely complex. I would say that if caching plugin does not work in this case then it is a problem of caching plugin. Let them figure out a solution. Many other plugins can alter query result like we do, and I am sure they do not think about persistent cache either. Caching plugin must have its own ways around.

Did you try the latest version? Does it work for you except persistent caching? Press Download button at plugin page: https://github.com/qTranslate-Team/qtranslate-x, which runs this link https://github.com/qTranslate-Team/qtranslate-x/archive/master.zip

GitHub has become one of the most popular standard for doing shared development. You will have to get engaged with them in any case for many different reasons, if you are in this line of business. Fortunately, they have pretty good documentation. Shortly: you fork the repository, clone it to your local computer, modify the code, push modifications to your fork, then from your fork it is two clicks to submit pull request here. You will find documentation on each step.

Please, try the latest version before anything else ;)

[herrvigg]

Comment by side777
Saturday Feb 14, 2015 at 18:50 GMT

---

any query var from the default list

add this to a meta query:
array( 'key' => 'qtranxf_language_' . qtranxf_getLanguage(), 'compare' => 'NOT EXISTS' );

[herrvigg]

Comment by side777
Saturday Feb 14, 2015 at 19:03 GMT

---

i use w3tc, which is very common. afaik most caching plugins make the wp cache persitent, btw.

What if one user submits new comment? It is not going to show up for anyone until cache is invalidated?

exactly. that's why the core invalidates the cache on any new comment. read the code.

it is a problem of caching plugin.

definitely not. i'd say it's a flaw in the core because it doesn't check for altered clauses. the caching plugin has no chance at all to handle this because it simply builds up upon the core cache function.

Many other plugins can alter query result like we do

wrong - i guess this approach is pretty specific: dynamically alter the SQL for not logged in users and for the exact same URL? i can't think of one plugin that alters the SQL to output different content at the same URL.

[herrvigg]

Comment by johnclause
Friday Feb 20, 2015 at 21:59 GMT

---

I am sorry, @side777, about the delay in response. I got busy now with other things. Could you meanwhile explain the logic behind the following:

add this to a meta query:
array( 'key' => 'qtranxf_language_' . qtranxf_getLanguage(), 'compare' => 'NOT EXISTS' );

[herrvigg]

Comment by side777
Monday Feb 23, 2015 at 13:50 GMT

---

^^ this makes the query individual for every language and different cache data is saved.
in a better future version you would save the languages used in a post in that meta key, compare 'LIKE' and skip the dirty manipulation of the query string, i.e. increase the performance of the query, PLUS avoid the cache problem.

[herrvigg]

Comment by johnclause
Monday Feb 23, 2015 at 19:31 GMT

---

I understand that it makes key unique and helps caching, I am curious how does this affects the final query and why it is safe to implement. I can look it up, but I thought you would explained it to me since you already did the investigation.