Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Menus API #278

Closed
jasonHav opened this issue Sep 27, 2018 · 20 comments · May be fixed by #302
Closed

Menus API #278

jasonHav opened this issue Sep 27, 2018 · 20 comments · May be fixed by #302

Comments

@jasonHav
Copy link

Hey guys,

Is there any consideration to adding an API endpoint to retrieve menus? Combined with the Wagtail v2 API, I can see a great toolset to make React/Angular/Vue driven Wagtail sites.

@ababic ababic modified the milestone: 2.11 Sep 27, 2018
@ababic
Copy link
Collaborator

ababic commented Sep 27, 2018

Hi @jasonHav! As it happens, I've had this in mind for a while :) I've already started laying some of the groundwork for it in this release (e.g. #276, and #279). I got pretty far in a local branch, but the changes were a little too 'sweeping', so I've decided to take a more incremental approach, improving other things as I go.

@ababic
Copy link
Collaborator

ababic commented Dec 2, 2018

Hi @jasonHav. I've made quite a bit of progress on a Menus API implementation, and I'm now seeking feedback from the community to help make sure the implementation makes sense / feels natural. Would you be willing to help review the state of the implementation so far?

It's PR #302.

@jasonHav
Copy link
Author

jasonHav commented Dec 2, 2018

Looking good! I'll find a project to have a play with it and get you any feedback I can.
Quickly reading over the code - what was the motivation behind using rest_framework APIView vs. Wagtail's BaseAPIEndpoint?

@ababic
Copy link
Collaborator

ababic commented Dec 2, 2018

Great, thanks @jasonHav. If you need any help getting things to work, let me know.

What was the motivation behind using rest_framework APIView vs. Wagtail's BaseAPIEndpoint?

The short answer there is that there's basically very little code in BaseAPIEndpoint that's applicable to what these menu generator views are doing. As far as I can tell, there's really nothing in Wagtail or DRF that seemed like a good fit in terms of functionality. APIView seemed like the simplest possible starting point.

@sowinski
Copy link

@ababic
Do you plan to release this feature soon?

I actually want to write a Vue based application and this kind of API would be great. (At least to have a serializer which I can use on my own API endpoint)

@ababic
Copy link
Collaborator

ababic commented Jul 10, 2019

@sowinski #302 is pretty far along, but I made some big changes recently, and a lot of tests need reworking. I still need to find a good way to allow custom field values to be rendered for sub-pages. I am going on holiday very soon though, so progress will be stalled for a while.

@sowinski
Copy link

Thank you for the update. In the meanwhile I will just write my own serializer :)

@sowinski
Copy link

sowinski commented Jul 11, 2019

I am trying to iterate over the main menu.
MainMenu.objects.all()[0].menu_items.all()
My problem is, that the returned objects are "MainMenuItem" but I need my custom "CustomMainMenuItem".

I tried to read your code but it is not easy for a beginner :D

EDIT:
Found finally something:
MainMenu.objects.all()[0].get_raw_menu_items()

With that method I get the first level. How can I get the next level of an menu item?

@ababic
Copy link
Collaborator

ababic commented Jul 11, 2019

@sowinski. Menu items aren't really designed to be recursively queried in this way, I'm afraid. Menu instances usually prefetch all of the page data they need in advance of rendering, and sub menus request their page data from the original menu instance. This pattern works well for rendering menus from template tags, but it's tricky to make use of it directly like this.

I'm afraid all I can do is recommend that you take a look at the views and serializers from #302, and see if you can work things out from there.

@sowinski
Copy link

@ababic
I just wrote a hacky solution and cache the results. If someone is changing the menu I will just refresh the cash and this is it for now.

If you need help (testing, feedback, examples etc.) let me know.

@onno-timmerman
Copy link

Any news on the API progress? Just starting on a project with VUE and wagtail and this would be great to use along.

@ababic
Copy link
Collaborator

ababic commented Nov 11, 2019

@onno-timmerman I've simply not had the time to look any closer at it for a few months. Where the PR is right now is: It works, but I haven't really figured out what I want to do in terms of caching/cachability, which I feel is important element. It's easy to add template fragment caching when using the template tag approach, but you kinda lose that option when you take the modern js / API route... unless the API has something built in.

@lalasmuathasim
Copy link

Using this I'm getting an error

ValueError at /wagtailmenus_api/v1/main_menu/
'MainMenuGeneratorArgumentForm' has no field named 'current_site'.

@noobmaster19
Copy link

Any updates on this feature?

@Mardik
Copy link

Mardik commented Sep 24, 2020

how did you get around this situation? did you use normal wagtail pages to structure the menus?

@viirak
Copy link

viirak commented Sep 5, 2021

Has this feature been merged?

@ababic
Copy link
Collaborator

ababic commented Sep 6, 2021

@viirak no, still a work in progress

@happygrizzly
Copy link

Hi everyone! Is it still a WIP?

@ababic
Copy link
Collaborator

ababic commented May 30, 2023

Abandoned now, sorry.

@MrCordeiro
Copy link
Contributor

Hey folks, so I'm going ahead and closing the issue. I feel it's stale now. Feel free to open a new issue if you still require this feature.

@MrCordeiro MrCordeiro closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants