Code

Opened 6 years ago

Closed 4 years ago

#6932 closed (fixed)

Flatpages does not provide a list of available flat pages to the context

Reported by: Dmitri Fedortchenko <zeraien@…> Owned by: faldridge
Component: Contrib apps Version: master
Severity: Keywords: flatpages list templatetag
Cc: mmitar@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I want to be able to create a menu of all available flat pages. So I wrote this patch that puts a list of flatpages into the context.

It is registration sensitive and does not list restricted pages if user is not logged in.

	{% for page in flatpages %}
		<h3><a href="{{page.url}}">{{page.title}}</a></h3>
	{% endfor %}

Attachments (5)

flatpages_list.diff (1003 bytes) - added by Dmitri Fedortchenko <zeraien@…> 6 years ago.
Put a list of flatpages into the context.
6932-template-tag-for-flatpages.diff (8.1 KB) - added by Mnewman 6 years ago.
Patch for a new template tag, this patch also includes tests for flatpages in general (on top of the test for the new code) and documentation
6932-template-tag-for-flatpage-2.diff (11.1 KB) - added by faldridge 4 years ago.
6932-template-tag-for-flatpages-2.diff (11.1 KB) - added by faldridge 4 years ago.
6932.diff (10.7 KB) - added by DrMeers 4 years ago.
Fix template logic and wording in example usage, DRY out exceptions, etc. (NB: uploaded after RFC)

Download all attachments as: .zip

Change History (22)

Changed 6 years ago by Dmitri Fedortchenko <zeraien@…>

Put a list of flatpages into the context.

comment:1 Changed 6 years ago by mattmcc

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

This probably isn't something that belongs in the app itself. I'd suggest instead using a template tag.

comment:2 Changed 6 years ago by Mnewman

  • Keywords templatetag added
  • Needs documentation set
  • Needs tests set
  • Owner changed from nobody to Mnewman
  • Patch needs improvement set
  • Status changed from new to assigned

I do like this idea as a templatetag. I will try my hand at it while waiting for a design decision.

comment:3 Changed 6 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

I'd be +1 for a template tag -- bonus points if it lists flatpages with a given prefix (something like {% get_flatpage_list [prefix <prefix>] as <varname> %}. Seems silly to always have in the context, though.

Changed 6 years ago by Mnewman

Patch for a new template tag, this patch also includes tests for flatpages in general (on top of the test for the new code) and documentation

comment:4 Changed 6 years ago by Mnewman

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Good to see this accepted. Wasn't too much work so, I just did it quick. It's good to know that I was on the same wavelength as Jacob, and scored the bonus points! Decided against the filtering for registration, because it is an easy boolean test and might limit some cases. This is documented, in the new documentation.

comment:5 Changed 6 years ago by Mnewman

  • Owner Mnewman deleted
  • Status changed from assigned to new

comment:6 Changed 6 years ago by Mnewman

  • milestone set to post-1.0

comment:7 Changed 6 years ago by antonius

I'd love to use this, though it doesn't seem to be present in the current trunk?

comment:8 Changed 6 years ago by Mnewman

  • Needs documentation set
  • Owner set to Mnewman
  • Status changed from new to assigned

antonius; nice thing about template tags is they can live your own code. Just copy out the code from templatetags.flatpages and put it in template tags in your own projects and it should just work. Or if you would prefer, I am sure this patch will get accepted faster when someone other than myself tests it out, patch it into your django trunk and report back on how it goes. I know right now the documentation doesn't match the refactor so those will need to be updated before being merged (I also just noticed a few typos).

comment:9 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:10 Changed 4 years ago by faldridge

Mnewman,

Are you still working on this? And if so, do the tests pass for you? They don't for me.

I need this exact functionality, so I'd like to pick up this ticket with your permission and get the tests working and patch up the documentation changes to meet the Django documentation guidelines.

comment:11 Changed 4 years ago by Mnewman

  • Owner Mnewman deleted
  • Status changed from assigned to new

It's all yours if you want to spend time on it.

comment:12 Changed 4 years ago by faldridge

  • Owner set to faldridge

comment:13 Changed 4 years ago by faldridge

  • Needs documentation unset

OK, the final patch, 6932-template-tag-for-flatpage-2.diff should be ready for merging.

  • The tag implementation no longer saves a queryset on the node to ensure thread safety.
  • All unit tests pass using django's runtests script.
  • Documentation changes now use more semantic markup.
  • Patch applies cleanly to trunk (r12843).

I know everybody is busy right now, but could someone with privileges please merge this or let me know what else is needed?

Changed 4 years ago by faldridge

Changed 4 years ago by faldridge

comment:14 Changed 4 years ago by mitar

+1

comment:15 Changed 4 years ago by mitar

  • Cc mmitar@… added

comment:16 Changed 4 years ago by justinlilly

  • Triage Stage changed from Accepted to Ready for checkin

Appears to adhere to what Jacob wanted as well as providing the necessary docs + tests. Applies cleanly to trunk and tests pass for sqlite.

Changed 4 years ago by DrMeers

Fix template logic and wording in example usage, DRY out exceptions, etc. (NB: uploaded after RFC)

comment:17 Changed 4 years ago by russellm

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

(In [13654]) Fixed #6932 -- Added a template tag that gives a list of available flatpages for a given user. Thanks to Dmitri Fedortchenko for the suggestion, and to Mnewman, faldridge and Simon Meers for their work on the patch.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.