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

Remove node cache to improve cache invalidation for config pages #168

Merged
merged 3 commits into from
Jul 16, 2024

Conversation

pookmish
Copy link
Contributor

@pookmish pookmish commented Jul 15, 2024

READY FOR REVIEW

Summary

  • Remove node-cache implementations
  • We are using the on demand cache invalidation on the route /api/revalidate but this isn't clearing the node-cache. We don't really need it anymore anyways. We can just eliminate it.

Copy link

vercel bot commented Jul 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
su-library ✅ Ready (Inspect) Visit Preview Jul 16, 2024 4:55pm

@imonroe
Copy link
Contributor

imonroe commented Jul 16, 2024

Ok, so I think I see what you're doing here, but a couple of things.
Something in the main menu styling has changed such that :hover and :active states seem to make the text disappear. I think it's turning the text white.
But more to the point, it doesn't seem to fix the problem with the Global Message revalidation.
Try this:

  • while on this branch, with the front-end running, set up a Global Message in Drupal. Enable it, save it.
  • Refresh the front end page in your browser. Note that the Global Message does not appear.
  • do a GET for http://localhost:3000/api/revalidate/?slug=/tags/config-pages&secret=xxxxx
  • verify that it has cleared out the config-pages cache.
  • Refresh the page on the front end. Note the Global Message now appears. It works the other direction too; if you then disable the global message, it will still appear in the front end until you do a manual revalidation.

I looked in the next module on the drupal side, and it looks set up to invalidate the config-pages cache tag correctly:
image

And when I do the revalidation manually, it works immediately. But it does not work when the config page is saved in the admin interface, which would be the expected behavior.

@imonroe
Copy link
Contributor

imonroe commented Jul 16, 2024

maybe this is something that needs to be fixed on the Drupal side, rather than the Next side. Maybe with an event listener on the global message update triggering the on-demand revalidation?

@pookmish
Copy link
Contributor Author

The config page invalidation works correctly as it should in both drupal and on this branch. I have no issues at all when editing/disabling/enabling the global messages banner. I even put in a console.log on the api/revalidate route and it shows the correct cache tags are being invalidated. I am confident in the removal of node cache fixing the issue.
If you want to test this out, make sure you are running yarn preview, not yarn dev. The latter behaves differently than the former in terms of cache functionality. This PR will solve the issues

@pookmish
Copy link
Contributor Author

As for the menu styling, yeah I'll fix that real quick.

@imonroe
Copy link
Contributor

imonroe commented Jul 16, 2024

Ok, so I got the same results on my local with yarn preview as well as yarn dev (didn't work). I tried it using both chrome and firefox with no differences.
I tried it on Gitpod, and I got the same results there (didn't work), presumably because it's doing a yarn dev for those environments.
So then I tried it on the Vercel preview build, and there it worked exactly as expected.

I'm not sure what's up with that. I'm going to do some troubleshooting on my local and see if something is messed up.

Anyways, since it works on Vercel, that's the important part.

@pookmish pookmish merged commit 7c066b4 into 1.x Jul 16, 2024
5 checks passed
@pookmish pookmish deleted the remove-node-cache branch July 16, 2024 18:05
imonroe added a commit that referenced this pull request Sep 11, 2024
* SUL23-540: Updated robots.txt to disallow /all/ (#166)

* Better automated tests & Gitpod by building a cloned site (#165)

* Remove node cache to improve cache invalidation for config pages (#168)

* SUL23-515 SUL23-541: Colors, fonts, word fixes to the toggles. (#169)

* SUL23-515 SUL23-541: Colors, fonts, word fixes to the toggles.

* Keyword search filter stubbed out (#156)

* Keyword search filter stubbed out

* SUL23-503: fixup to search box styles

* fixup to styles

* moved over the search cursor

* Fix label

* fixup to the button toggles

---------

Co-authored-by: Jen Breese-Kauth <[email protected]>

* SUL23-525: Updates to the display for Med breakpoint on Branches and Centers (#170)

* SUL23-525: stashing work.

* SUL23-525: using grid to get alignment and display for md view

* Changed three tabular layouts to use the grid at the md breakpoint

* fixup to comment

* SUL23-438: Moved the location link and changed to Open or Closed until (#163)

Co-authored-by: Mike Decker <[email protected]>

* SUL23-479: added space (#176)

* SUL23-549: places to study alignment (#172)

* SUL23-549: places to study alignment

* SUL23-570: adding the corrected times to the places to study table (#175)

* SUL23-550: Move the condition so the td element remained even when empty (#171)

* SUL23-550: Move the condition so the td elmement remained even when empty

* fixup

* SUL23-548 SUL23-553 SUL23-554 SUL23-555 SUL23-556 SUL23-547 (#173)

* SUL23-548 SUL23-553 SUL23-554 SUL23-555 SUL23-556 SUL23-547

* fixup

* fixup

* fixup

* SUL23-543: rounded corner

* SUL23-557: adding aria-hidden and tabindex for a11y to photos on tabular data tables (#177)

* SUL23-557: stashing work to ask about tomorrow

* SUL23-557: fixup for the tabindex

* fixup to branch  page

* SUL23-557: removed the link on the image in the study places

* Adjust how library hours data is cached to reduce cache writes/reads (#178)

* SUL23-503: search label font size (#180)

* SUL23-570: changed address to size 16 (#179)

* SUL23-580: nav fixup (#184)

* SUL23-556: normal font and leading (#186)

* SUL23-503 Add text reset button and no results display (#185)

* SUL23-503 Add text reset button and no results display

* SUL23-503: changed size, added padding and color

* fixup after merge

---------

Co-authored-by: Jen Breese-Kauth <[email protected]>
Co-authored-by: Ian Monroe <[email protected]>

* SUL23-577 SUL23-586: buttons and vertical centering on tables (#183)

* SUL23-577 SUL23-586: buttons and vertical centering on tables

* fixed the col spacing

---------

Co-authored-by: Ian Monroe <[email protected]>

* SUL23-576 SUL23-575 SUL23-574: fixed height of toggle and filters (#182)

* SUL23-576 SUL23-575 SUL23-574: fixed height of toggle and filters

* fixup to corner

* SUL23-576: fixup

* removed rounded idea

* SUL23-591: added the th row=scope for a11y (#187)

Co-authored-by: Ian Monroe <[email protected]>

* Fix date formatting when the input is UTC

* SUL23-551: sentence case for branches and places to study (#188)

* SUL23-551: sentence case for branches and places to study

* Sentence case for all locations

* SUL23-482: set Todays hours title to h2 (#189)

* SUL23-482: set Todays hours title to h2

* "Hours" needed to be sentence case

* SUL23-587 SUL23-589 SUL23-590 (#190)

* SUL23-587 SUL23-589 SUL23-590

* adding the correct padding after cell

* SUL23-588: make the hover hug the phone and email

* Moved to just lg for inline flex

* fixup

* SUL23-578: fixup to the margin-bottom and casing (#191)

* SUL23-578: fixup to the margin-bottom and casing

* adding the flex change from the parent branch

* Fix date formatting when the input is UTC

* SUL23-578: added the 2 digits to the closed until time and lowercase of until

* fixup to the AM PM

---------

Co-authored-by: Mike Decker <[email protected]>

---------

Co-authored-by: Mike Decker <[email protected]>

* SUL23-577: fixed button height. removed include not used (#192)

* SUL23-587: updates to the people table (#193)

* SUL23-587: updates to the people table

* SUL23-440 Add "Load More" button on news lists over 30 items

* Added missing fields for people teasers

* SUL23-440 Adjusted "Load More" to "Load more"

* SUL23-597 Fix "today" vs "tomorrow" vs "friday" next opening day

* SUL23-597 Fix hour displays for next open date/time (#194)

* Removed "Hours this week" text

* SUL23-598: fixup up hover on branch name (#195)

* SUL23-598: fixup up hover on branch name

* fixup

* SUL23-574: setting exact height (#196)

* SUL23-580 SUL23-538: Added in the at to the time string (#197)

* Handle empty response from api/library-hours if something fails

* SUL23-585: Adding Donate now button to header (#198)

* SUL23-585: Adding Donate now button to header

* SUL23-585: button in header

* fixup

* changed Links to links

* SUL23-585: Back to digital red (#201)

* SUL23-571 Sort library options by title

* SUL23-600 show closing minutes if they are not 00 (#202)

* SUL-23-570: adding Hours this week for default (#203)

* SUL23-603: removed padding-left on branch name (#205)

* SUL23-604: tweaking the toggle spacing (#206)

* SUL23-573: tightening up spacing (#199)

* SUL23-573: tightening up spacing

* fixup

* fixup for the hover on email and phone

* fixup

* adjusting the top and bottom spacing and removed empty spacers

* fixup

* SUL23-608: changed the padding-left when the browser size is smaller (#207)

* fixup for the extra space

* fixup

* SUL23-605: fine tuning the sul people page. (#204)

* SUL23-605: fine tuning

* src/

* asking for feedback

* Fix on click handlers

* fixup to the group classes

* adding hover and active

* Added checkmark visual indicator

---------

Co-authored-by: Mike Decker <[email protected]>

* SUL23-601: changed when focused to not be red with red underline (#208)

* SUL23-601: changed when focused to not be red with red underline

* fixup

* SUL23-601: fixed the All to All specialists per slack rquest.

* SUL23-609: fix to text in hours card on homepage (#209)

* SUL23-609: fix to text in hours card on homepage

* fixup

* Sul23 611  Improve and simplify toggles (#210)

* Add a clear cache button for preview environments

* SUL23-610 add SDR as an oembed provider in the wysiwyg

* Sul23 611  tweaks (#211)

* SUL23-611: draft work for spacing on small screens

* adjustments to the focus and toggles

* tweaks

* tweaks

* fixup

* fixup

* fixup

* fixup

* Removed old eslint file

* SUL23-614 SUL23-615: fixes for html validation (#213)

* SUL23-615: footer link size (#212)

* SUL23-617: changed to Expertise (#214)

* Updated changelog

---------

Co-authored-by: pookmish <[email protected]>
Co-authored-by: Jen Breese <[email protected]>
Co-authored-by: Mike Decker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants