Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#2119 closed change (fixed)

[CMS] Add a way to retrieve contents of a different page

Reported by: saroyanm Assignee: kzar
Priority: P3 Milestone:
Module: Sitescripts Keywords:
Cc: trev, sebastian Blocked By:
Blocking: #2035, #2118 Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: yes
Review URL(s):

http://codereview.adblockplus.org/5148261828526080/

Description (last modified by trev)

Background

As mentioned in #2118, sometimes we have a page list the topics covered on another page. A typical example is https://adblockplus.org/en/faq, each section refers to a different page.

Currently, we already have the toclist template filter that is used to generate the Table of Contents for a page. However, it only accept HTML code as parameter (available to default.tmpl as the body variable), and there is no way to retrieve the contents of a different page.

What to change

Add a template function get_page_content(page, locale=None) that will process a different page similarly to cms.utils.process_page() function but return the head and the body without embedding them in a template. Note that cms.utils.process_page() logic isn't quite trivial, so it shouldn't be duplicated.

Change History (20)

comment:1 Changed 5 years ago by saroyanm

  • Component changed from Infrastructure to Sitescripts

comment:2 Changed 5 years ago by saroyanm

  • Summary changed from [adblockplus.org Anwiki to CMS migration] toclist to accept relative path to [CMS] toclist to accept relative path

comment:3 follow-up: Changed 4 years ago by sebastian

  • Cc sebastian added
  • Verified working unset

So you want to show a table of contents for page A on page B? Sounds like an uncommon scenario to me. On which pages do we do that currently? Why do we have to do so?

comment:4 in reply to: ↑ 3 ; follow-up: Changed 4 years ago by saroyanm

Replying to sebastian:

On which pages do we do that currently?

Here are the list of pages affected by that implementation:
https://adblockplus.org/en/faq
https://adblockplus.org/en/android-about
https://adblockplus.org/en/android-config

comment:5 in reply to: ↑ 4 ; follow-up: Changed 4 years ago by sebastian

Replying to saroyanm:

Replying to sebastian:

On which pages do we do that currently?

Here are the list of pages affected by that implementation:
https://adblockplus.org/en/faq

There is no TOC here.

https://adblockplus.org/en/android-about
https://adblockplus.org/en/android-config

The TOC here lists the content of the same page. Shouldn't that work with the toclist filter as it is currently implemented?

comment:6 in reply to: ↑ 5 Changed 4 years ago by saroyanm

Replying to sebastian:

Replying to saroyanm:

Replying to sebastian:

On which pages do we do that currently?

Here are the list of pages affected by that implementation:
https://adblockplus.org/en/faq

There is no TOC here.

Here is the content of that page:
<anwtoc titletag="h2" page="en/faq_project"/>
<anwtoc titletag="h2" page="en/faq_install"/>
<anwtoc titletag="h2" page="en/faq_basics"/>
<anwtoc titletag="h2" page="en/faq_features"/>
<anwtoc titletag="h2" page="en/faq_customization"/>
<anwtoc titletag="h2" page="en/faq_internal"/>
<anwtoc titletag="h2" page="en/faq_troubleshooting"/>

https://adblockplus.org/en/android-about
https://adblockplus.org/en/android-config

The TOC here lists the content of the same page. Shouldn't that work with the toclist filter as it is currently implemented?

As far as I can see they are listing content from https://adblockplus.org/en/android-faq:
<anwtoc page="en/android-faq"/>

comment:7 Changed 4 years ago by sebastian

  • Ready set

Ah, now I get what you mean. You aren't talking about the actual TOC list at the top of the page, but the FAQ sections which list items from the respective FAQ page.

comment:8 Changed 4 years ago by sebastian

  • Priority changed from Unknown to P3

comment:9 Changed 4 years ago by trev

  • Ready unset

Sorry about unsetting the "ready" flag but I think that this is the wrong approach. The toclist filter already works on any content - what is needed here is a new template filter to load the content for another page. Ideally it would reuse some code with utils.process_page() which currently does everything necessary but also applies the overall template. We should instead stop at getting the head and body of the page.

comment:10 Changed 4 years ago by sebastian

I already though about that. But didn't have a strong preference on one way or another here .If in doubt I like to keep templates as limited as possible. But fair enough.

comment:11 Changed 4 years ago by trev

  • Description modified (diff)
  • Summary changed from [CMS] toclist to accept relative path to [CMS] Add a way to retrieve contents of a different page

Updated title and description, does that sound right to you?

comment:12 Changed 4 years ago by sebastian

  • Ready set

comment:13 Changed 4 years ago by kzar

  • Owner set to kzar

comment:14 Changed 4 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:15 Changed 4 years ago by kzar

Your idea seems to work OK Wladimir, I was able to get the toclist for a page with a snippet like {{ get_page_content("about.html")[1]|toclist }}. Only thing I noticed is that we might want to move the toc macro out of default.tmpl as we don't seem to have access to it from the other templates currently.

comment:16 Changed 4 years ago by sebastian

Maybe we should just make the toclist filter generate HTML, getting rid of the macro.
The HTML generated there is rather generic (just a <ul>, no site-specify classes/ids).

comment:17 Changed 4 years ago by kzar

Might be a good idea, one problem would be that the toclist filter would then need to know the page's URL to prepend for each link in the table of contents. We could always add that as an extra parameter though I suppose.

comment:18 Changed 4 years ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:20 Changed 4 years ago by Ross

  • Verified working set

Implemented and working.

Tested by:
Running the website converter script as per the read me and checking the TOC output for the mentioned pages + serving pages from CMS.

Tested with:
www / r18024
sitescripts / r501
web.adblockplus.org / r68
website-converter / master / 105208883c4336a3788820fe9ab379cf639f8387

Note: See TracTickets for help on using tickets.