Page MenuHomePhabricator

Invalidate cache when users extend or renew a collection
Closed, ResolvedPublic

Description

Users can Renew/Extend a collection from My Library, which returns them back there afterwards. At the moment the cache for that page isn't invalidated when this happens, however, which might be confusing - users come back to see the same status on that collection (i.e. the ability to extend it).

Event Timeline

jsn.sherman changed the task status from Open to In Progress.Apr 5 2022, 1:04 PM
jsn.sherman claimed this task.
jsn.sherman moved this task from Ready to In Progress on the The-Wikipedia-Library (Kanban) board.

We could also be invalidating when users:

  • Apply for a collection
  • Return a collection

Remember when I said I should definitely be able to get this ready for review by the week's end? About that...

  • It turns out that I've already gathered a bit of rust on my django skills
  • I missed a day due to inclement weather and a power outage
  • As part of this task, the view needs to be reworked. Initially I was reworking it to use a form, but that turned out to be a bit of a fight since its use case is async javascript and json (AJAJ?), I'm switching directions to django rest

Okay, I've got a WIP here:
https://github.com/WikipediaLibrary/TWLight/pull/996

I was very much hoping to get this over the finish line today, but it took me longer than I anticipated to reacquaint myself with our very abstract application forms. It's really close!

@Samwalton9 while looking at the application process, I realized that while my work in progress invalidates the cache upon application submission, it doesn't do so upon approval/sent status change by the coordinator, which is a pretty important bit.
Handling the cache key (needed for invalidation) via forms has turned out to be pretty involved and is how I've spent the majority of my time on this so far. Because there can be some delay between app submission and approval, the cache key included in the form may be outdated anyway (eg. the user will probably have a newer cached version that would have a different cache key) I think I'm just going to add a cache key field for editors that gets updated when they load the my library page cold. that way we can just clear the cache key stored for the user without having to pass that state around via user-exposed urls. Even though we are adding back in a little bit of db usage, we should largely keep our performance benefits of caching while greatly reducing the amount of code needed to invalidate that cache.

That makes sense to me, also sounds like it would be easier to leverage in other situations if something occurs to us?

@Samwalton9 update:
I implemented the change in the pr, and there are a few items worth mentioning:

  1. clearing the my_library cache upon application submission doesn't seem to be all that useful; as far as I can tell, my_library doesn't change when an app is submitted, cache or no.
  2. the server side cache is working as expected, and now that we're actually invalidating it properly we can keep it around a lot longer, I'm playing with that value currently
  3. the client may still cache up to 5 minutes, meaning navigating to the page from within the site within 5 minutes of approval will show stale content. I'm tempted to just tell browsers not to cache the page at all since we can serve the page so cheaply out of the server-side cache

clearing the my_library cache upon application submission doesn't seem to be all that useful; as far as I can tell, my_library doesn't change when an app is submitted, cache or no.

I think I was specifically thinking about submitting an application renewal, which does alter what's displayed to users (they no longer see a Renew button).

okay, this turned into a really interesting exercise due to the way django couples server-side and browser-side caching.
I think this is ready, but I'm going to get this up on staging tomorrow morning CDT for additional testing.

pushing to staging was the right call here, as I needed to adjust some settings to allow JavaScript to be able to access the CSRF tokens from cookies. The fix is in the pipeline for staging so I can do followup testing

Tested this a bit on the My Library workflows and it seems to be working well!