An empty category (no nodes associated with the category) displays all tags (of different categories) in (all) refine blocks, using the relation "Refine tags with AND, interconnected through nodes"

CommentFileSizeAuthor
#9 refine_by_taxo-empty-cat-all-tags-2.patch1.25 KBmo6
#7 refine_by_taxo-empty-cat-all-tags-2.patch1.25 KBmo6
#2 refine_by_taxo-empty-cat-all-tags.patch2.22 KBmo6

Comments

Bèr Kessels’s picture

Bèr Kessels commented
Assigned: Unassigned » Bèr Kessels

acknowleged, thanks for the report.

mo6’s picture

mo6
he/him
Primary language Dutch
commented
Status: Active » Needs review
StatusFileSize
new refine_by_taxo-empty-cat-all-tags.patch2.22 KB

Fixed refine_by_taxo_find_related_terms_by_node() to return an empty array of terms when there are no nodes in the displayed category. Also added an optimization to the db query and fixed two typos. Tested and works in my setup.

Bèr Kessels’s picture

Bèr Kessels commented

Looks good, except of the one-line } else { :)

If ssomeone can please test and review, we can push this in sooner, i won"'t have time to do this myself.

@george: again thanks for the work, we can use this very well. However, I prefer it when patches aer dedicated. Even with small patches like these, try to keep them dedicated to one issue: for now leave this. But in future, best is to try and make three patches: typo-fix, db-optimisation, array-issue. This is common Drupal practice, because it has proven to work the best. Core will simply refuse a patch that does more then one thing.
But, as said: leave it for now, this patch is small enough :)

mo6’s picture

mo6
he/him
Primary language Dutch
commented

@Bèr: I just *knew* you'd notice the else line. ;) I know it's scary to submit a patch to core because of Dries whipping someone's *ss when indenting or line length is wrong. So for now I'll limit myself to patching small modules and still get whipped! Keep up the good work. ;)

mo6’s picture

mo6
he/him
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Patch works fine.

Bèr Kessels’s picture

Bèr Kessels commented
Status: Reviewed & tested by the community » Needs work

I am not happy with the else { (not talking about it being on one lie, this time).

To me it seems a hackish solution, please shift the if ($conditions) { part around a bit, instead, that will keep the code/logic a lot cleaner.

mo6’s picture

mo6
he/him
Primary language Dutch
commented
Version: 4.7.x-1.x-dev » 5.x-1.x-dev
Status: Needs work » Needs review
StatusFileSize
new refine_by_taxo-empty-cat-all-tags-2.patch1.25 KB

Updated patch against 5.x branch. Switched the conditions checks around.

Bèr Kessels’s picture

Bèr Kessels commented

For some weird reason I cannot open the patch. I thought that was a hiccup in the system two days ago, but I can still not open the patch. Care to upload it again, and be careful with spaces in the filename, I have a feeling that is causing the problem right now.

mo6’s picture

mo6
he/him
Primary language Dutch
commented
StatusFileSize
new refine_by_taxo-empty-cat-all-tags-2.patch1.25 KB

Strange. New try.

Bèr Kessels’s picture

Bèr Kessels commented
Status: Needs review » Reviewed & tested by the community

Looks good, changing status to RTBC.

mo6’s picture

mo6
he/him
Primary language Dutch
commented
Status: Reviewed & tested by the community » Fixed

Committed.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)