Add support for nested attributes in groupBy filter

[ogonkov]

Summary

Proposed change:

Add support for nested attributes access for groupBy filter to match Jinja2 behaviour

Closes #1198 .

Checklist

I've completed the checklist below to ensure I didn't forget anything. This makes reviewing this PR as easy as possible for the maintainers. And it gets this change released as soon as possible.

• Proposed change helps towards purpose of this project.
• Documentation is added / updated to describe proposed change.
• Tests are added / updated to cover proposed change.
• Changelog has an entry for proposed change (if user-facing fix or feature).

[codecov-io]

Codecov Report

Merging #1276 into master will decrease coverage by 0.11%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #1276      +/-   ##
==========================================
- Coverage   89.53%   89.41%   -0.12%     
==========================================
  Files          22       22              
  Lines        3020     3043      +23     
==========================================
+ Hits         2704     2721      +17     
- Misses        316      322       +6     

Impacted Files	Coverage Δ
nunjucks/src/lib.js	83.61% <75.00%> (-1.45%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec8eae5...a5cca7b. Read the comment docs.

[ogonkov]

@fdintino should i make PR against your dev branch aswell?

[anaurelian]

This is the code I'm using to achieve the same effect:

This is a filter I'm using for Nunjucks in Eleventy:

groupByEx: function (arr, key) {
    const result = {};
    arr.forEach(item => {
        const keys = key.split('.');
        const value = keys.reduce((object, key) => object[key], item);

        (result[value] || (result[value] = [])).push(item);
    });
    return result;
}

Sample (actual) usage:

for categoryName, articles in productArticles | groupByEx("data.categoryName")

Acknowledgement: Code based on whatterz/includes.js

[ogonkov]

Now it should work out of the box (when it gets merged)

[fdintino]

Don't worry about the conflicts, I'll take care of those. I'll try to set aside some time, if not today, then this week to give feedback or merge.

[ogonkov]

@fdintino can you start from reject PR, please? )

[fdintino]

Sure, will do.

[codecov-commenter]

Codecov Report

Merging #1276 into master will increase coverage by 0.08%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master    #1276      +/-   ##
==========================================
+ Coverage   89.56%   89.64%   +0.08%     
==========================================
  Files          22       22              
  Lines        3027     3043      +16     
==========================================
+ Hits         2711     2728      +17     
+ Misses        316      315       -1     

Impacted Files	Coverage Δ
nunjucks/src/lib.js	85.88% <94.11%> (+0.81%)	⬆️
nunjucks/src/filters.js	96.20% <100.00%> (ø)
nunjucks/src/compiler.js	95.86% <0.00%> (+0.34%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7087fa9...1e29863. Read the comment docs.

[ogonkov]

@fdintino this PR is ready for review ;)

[fdintino]

A small observation: the behavior here doesn't respect opts.throwOnUndefined. The previous implementation didn't either, so I don't think you need to address that in this PR. It does mean that it behaves differently from jinja2, where attempts to groupby a non-existent attribute raises an exception. It's regrettable that nunjucks doesn't have its own Undefined object, distinct from 'undefined', the way that jinja2 does.

If you did want to use the value of Environment.opts.throwOnDefined, you would be able to get it from this.env.throwOnUndefined in the filter function.

[ogonkov]

@fdintino i have add support for this option. I just throw it inside function, hope it would be handled somewhere above.

[ogonkov]

@fdintino everything is OK?

[fdintino]

Yep, everything is fine. Sorry, had to step away for a bit when I was in the middle of preparing for merge.