Make Menu.get_template neutral to template engine #190

hongquan · 2017-11-09T09:49:29Z

This is to make wagtailmenus work with Jinja2 template engine (#89).

The PR makes change in one internal API: The Menu.get_template() now return instance of Template class in django.template.backends, not django.template.base.Template class.

There is change in internal API: The `Menu.get_template()` now return the instance of Template class in `django.template.backends`, not `django.template.base.Template` class.

ababic · 2017-11-09T11:40:18Z

Hi @hongquan,

Thank you for this. It looks as though the Travis CI integration hasn't worked for this PR (I'm unsure why). Would you possibly be able to make a very minimal commit, and we'll see if Travis CI will do it's thing?

hongquan · 2017-11-09T12:22:14Z

I've just added 1 commit.

ababic · 2017-11-09T13:38:20Z

Hi @hongquan,

Travis CI is failing to build with a 'Abuse detected' message for some reason. I've just opened a ticket with their support team to see why this is happening.

ababic · 2017-11-10T10:21:28Z

Hi @hongquan,

I have the following feedback from the Travis team:

If a user- or the organization the pull request is coming from- hasn't ever used Travis before, our system will automatically flag the pull request. It looks like no user with push access to the AgriConnect org has used Travis- at least, not since that org was created. The solution here would be for the user who created the pull request to log into travis-ci.org and sync their account- after that, they should be good go, and if that doesn't work, they'll be in our system so we can manually flag it as not abusive.

Are you okay to try signing up for an account on travis-ci.org using you github account? It sounds like that might solve the problem.

Thanks :)

hongquan · 2017-11-12T12:11:37Z

I have just grant Travis CI access to AgriConnect. Hope it will help.

ababic · 2017-11-12T17:24:19Z

Thanks @hongquan. If you could just make another small commit, we'll see if it worked :) maybe just change your name in contributors, then commit a fix for it after?

hongquan · 2017-11-13T04:04:14Z

I have to do this in Travis CI to make it able to build:

codecov · 2017-11-13T04:06:39Z

Codecov Report

Merging #190 into master will decrease coverage by 0.07%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
- Coverage   97.08%   97.01%   -0.08%     
==========================================
  Files          73       73              
  Lines        2644     2644              
==========================================
- Hits         2567     2565       -2     
- Misses         77       79       +2

Impacted Files	Coverage Δ
wagtailmenus/models/mixins.py	`100% <100%> (ø)`	⬆️
wagtailmenus/models/menus.py	`97.3% <80%> (-0.36%)`	⬇️

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 bacdd86...f0aed87. Read the comment docs.

ababic · 2017-11-14T16:00:40Z

Hi @hongquan,

Thank you for your time in putting this together. I'm happy enough with the code changes here, but I'm not quite sure about making these additions to the documentation just yet - I think I would rather those be added separately as another pull, along with some other things, such as:

Adding a complete Jinja extension to wagtailmenus (making all tags available, not just flat_menu and sub_menu) so that users don't need to create their own.
Updating the test suite settings to support Jinja templates and make use of the extension.
Adding Jinja page/menu templates to the test app and testing the HTML output of all tags to ensure the output is as expected.

The instructions can then be simplified, and we'll have a working settings configuration that we can point developers to in case they'd rather look at / copy from a working implementation.

Does this sound okay to you?

hongquan · 2017-11-14T16:14:36Z

It is OK to me. Do I need to revert the commit of updating documentation?

ababic · 2017-11-14T16:22:48Z

@hongquan Yes, if you wouldn't mind :)

ababic · 2017-11-14T19:31:56Z

docs/source/rendering_menus/index.rst

@@ -7,3 +7,4 @@ Rendering menus

    template_tag_reference
    custom_templates
+    use_with_jinja2


Please remove this for now - We'll work on Jinja-specific stuff as a separate piece of work :)

ababic · 2017-11-14T19:32:20Z

docs/source/rendering_menus/use_with_jinja2.rst

+      {{ main_menu() }}
+    </div>
+
+


Please remove this file for now - We'll work on Jinja-specific stuff as a separate piece of work :)

This reverts commit 40a5b83.

hongquan · 2017-11-15T03:05:42Z

I removed the docs.

ababic · 2017-11-15T20:07:39Z

Big thanks @hongquan :) This has now been merged

Make Menu.get_template neutral to template engine

985fd29

There is change in internal API: The `Menu.get_template()` now return the instance of Template class in `django.template.backends`, not `django.template.base.Template` class.

hongquan mentioned this pull request Nov 9, 2017

Jinja2 extension #89

Open

Add me to CONTRIBUTORS

9ee9945

hongquan added 2 commits November 13, 2017 10:47

Add documentation about using with Jinja2 template.

40a5b83

More explanation in documentation about Jinja2

41b361c

ababic requested changes Nov 14, 2017

View reviewed changes

Revert "Add documentation about using with Jinja2 template."

f0aed87

This reverts commit 40a5b83.

ababic approved these changes Nov 15, 2017

View reviewed changes

ababic merged commit b2ca4cd into jazzband:master Nov 15, 2017

ababic mentioned this pull request Nov 19, 2017

Revert "Make Menu.get_template neutral to template engine" #191

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Menu.get_template neutral to template engine #190

Make Menu.get_template neutral to template engine #190

hongquan commented Nov 9, 2017

ababic commented Nov 9, 2017 •

edited

Loading

hongquan commented Nov 9, 2017

ababic commented Nov 9, 2017 •

edited

Loading

ababic commented Nov 10, 2017

hongquan commented Nov 12, 2017

ababic commented Nov 12, 2017

hongquan commented Nov 13, 2017

codecov bot commented Nov 13, 2017 •

edited

Loading

ababic commented Nov 14, 2017

hongquan commented Nov 14, 2017

ababic commented Nov 14, 2017

ababic Nov 14, 2017

ababic Nov 14, 2017

hongquan commented Nov 15, 2017

ababic commented Nov 15, 2017

Make Menu.get_template neutral to template engine #190

Make Menu.get_template neutral to template engine #190

Conversation

hongquan commented Nov 9, 2017

ababic commented Nov 9, 2017 • edited Loading

hongquan commented Nov 9, 2017

ababic commented Nov 9, 2017 • edited Loading

ababic commented Nov 10, 2017

hongquan commented Nov 12, 2017

ababic commented Nov 12, 2017

hongquan commented Nov 13, 2017

codecov bot commented Nov 13, 2017 • edited Loading

Codecov Report

ababic commented Nov 14, 2017

hongquan commented Nov 14, 2017

ababic commented Nov 14, 2017

ababic Nov 14, 2017

Choose a reason for hiding this comment

ababic Nov 14, 2017

Choose a reason for hiding this comment

hongquan commented Nov 15, 2017

ababic commented Nov 15, 2017

ababic commented Nov 9, 2017 •

edited

Loading

ababic commented Nov 9, 2017 •

edited

Loading

codecov bot commented Nov 13, 2017 •

edited

Loading