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

Scrap this plugin? #14

Open
jechols opened this issue Nov 14, 2019 · 1 comment
Open

Scrap this plugin? #14

jechols opened this issue Nov 14, 2019 · 1 comment

Comments

@jechols
Copy link
Contributor

jechols commented Nov 14, 2019

Yes, scrap it. I said it, it's out there, you're all thinking about it now.

Why?

This plugin is trying to do way too many totally different things

  • In the simplest case: provide curated newspaper pages at your site's /featured_content URL
  • Grab a random newspaper page that has the same date as today
  • Grab a totally random newspaper page and feature it
  • Offer APIs to do any of the above, but return page objects in code which can be used in overridden or custom templates of a theme
  • It specifically force-seeds the RNG to ensure randomness is not truly random
    • This avoided the problem where a refresh loaded an entirely different "Today in history" page, busting various image server caches. But it added new problems, particularly since some people actually enjoyed that feature. But now that it's in the plugin and some people rely on this, everybody using the plugin is forced to do it this way.

These seem similar from the outside, but the code just ends up being a lot of "if curated, do this and ignore all the other code" types of logic. Which means everybody who uses the plugin is actually only using one small part of it.

The solution

  • Add a function in core that allows searching pages by partial date - specifically, month and day without regard to year. That is a fairly discrete function which removes the need for the API part of this plugin on the UO side.
  • Add a "featured page" template in core that can be used to render any arbitrary newspaper page with generic enough styling that it can be used anywhere - homepage, sub-page like /featured_content, wherever. This should allow for custom captions, too.
    • It looks like django's include allows passing context, so this should be pretty easy to do
  • Add some easy "finder" in core to replace this plugin's approach to finding a page from the PAGES configuration (https://github.com/open-oni/plugin_featured_content/blob/master/helpers.py#L103-L127). This kind of functionality should already exist in some form anyway, since we have to parse a URL with the same data and turn it into a page object.

With all this stuff done, a plugin like this becomes unnecessary. But it will require some documentation. Right now people building themes have no idea what functions exist for theme-related code. ONI needs documentation that can tell people what's in core that they can use as well as how they could make use of it, and these new functions could be the first-documented "public API for themes" functions.

@jduss4
Copy link
Contributor

jduss4 commented Nov 14, 2019

Would we need a "get random" type function as well? I like this approach better than the rather stretched plugin....amazing that such a small thing can be attempting to do such much all at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants