Skip to content

Conversation

@eyalacato
Copy link
Collaborator

No description provided.

Copy link
Member

@richardkorthuis richardkorthuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omdat mijn comment gaat over een stuk code wat er al in zat, approve ik de PR wel, maar eigenlijk zou er nog wel een aanpassing nodig zijn.

if ( class_exists( \Caching::class ) ) {
\Caching::get_instance()->delete_cache_by_endpoint( '%/openagenda/v1/items', \Caching::FLUSH_LOOSE, true );
if ( class_exists( Caching::class ) ) {
Caching::get_instance()->delete_cache_by_endpoint( '%/openagenda/v1/items', Caching::FLUSH_LOOSE, true );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoewel ik zie dat je de code alleen iets aanpast en het er dus al in zat: Dit lijkt mij heel onwenselijk. Als een event nu wordt aangepast flush je alle caches van openagenda/v1/items, terwijl dat er veel meer kunnen zijn dan alleen caches met deze specifieke post er in. Het flushen van de cache na een save_post kun je volledig aan de WP REST Cache (icm de OWC addon) overlaten. Die detecteert als het goed is alle relaties in de caches en weet daarmee precies welke caches er geflusht moeten worden (en vooral welke NIET hoeven te worden geflusht).

Het lijkt mij dus dat dit stukje code helemaal weg kan en wordt afgehandeld door de WPRC + addon. Indien die dat niet goed doet, moet je kijken naar de process_cache_relations functie van de OpenAgendaCache in de addon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@richardkorthuis : Ik heb de code om caching te flushen in deze plugin verwijderd. Ik heb het lokaal getest en dit wordt inderdaad afgehandeld via de WP Rest cache OWC Add-on.

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

Successfully merging this pull request may close these issues.

2 participants