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

Status of the App is not updated immediately on the App list #430

Closed
csisztaiarnold opened this issue Jun 24, 2020 · 4 comments · Fixed by #437
Closed

Status of the App is not updated immediately on the App list #430

csisztaiarnold opened this issue Jun 24, 2020 · 4 comments · Fixed by #437
Assignees
Labels
bug Something isn't working
Milestone

Comments

@csisztaiarnold
Copy link

csisztaiarnold commented Jun 24, 2020

Description

When someone logs in as an authenticated user (not admin) into a site, and changes the status of an App's product in Apigee Edge's UI, the status will not change on the App list as well. Meanwhile, it's refreshed on the App's page itself.
This might be confusing for a user.
For a user who is logged in to the site as an administrator, this works as intended.
None of the cache settings (API Product caching or Developer App settings cache) affects the outcome.

Steps to Reproduce

Steps to reproduce the behavior:

  1. Create an App in Drupal, append a Product and a User to it.
  2. Log in to the site with the corresponding user and go to the App list.
  3. In Apigee Edge's UI, change any Product's status of the App (from Approved to Revoked or vice versa).
  4. Refresh the App list page.

Actual Behavior

The status in the App list remains unchanged, whilst if you click on the App's name and go to the App page itself, the status is changed. The status on the App list will only change if a global cache rebuild is applied.

Expected Behavior

Change the status of the App on the App list as well.

Screenshots

On the App list: https://i.imgur.com/ndKIA69.png (status is not changed)
On the App's page: https://i.imgur.com/mmpWq5A.png (status is changed)

Version Info

Apigee Edge Drupal integration Module 8.x-1.7, Drupal core 8.8.6

@csisztaiarnold csisztaiarnold added the bug Something isn't working label Jun 24, 2020
@mxr576
Copy link
Contributor

mxr576 commented Jun 25, 2020

Drupal does not handle if an entity can be edited outside of Drupal's database. Also, it seems entity list builders were being built for admin purposes and not for end-users. When an entity gets updated in Drupal, the [ENTITY_TYPE]_list tag automatically gets invalidated in caches, which invalidates cached entity list builder pages.

This is not enough here. We have an app cache that controls how long we should not ask Apigee Edge for changes basically. This is basically a cache max age for all places where an app is being rendered and it works on the app view, but not on the list pages, because Drupal's entity listing pages does not calculate with cache data related to listed entities.

We have a calculated "App state" rendered on the list builder and the render array should contain all cache data from the related app entity, including max-age.

My guess that this could be an issue on other entity listing pages, not just on apps.

A simple fix could be added the configured cache timeout to entity listing pages too.

@arlina-espinoza
Copy link
Contributor

arlina-espinoza commented Jun 30, 2020

Yeah, I also think that potentially all list builders might not be respecting the cache expiring time set in their respective apigee settings, not just for apps. We could add a max-age cache tag to the listing render array to enforce the cache settings, besides the [ENTITY_TYPE]_list tag . We did similar work for the team lists in this PR: #419

@shadcn
Copy link
Contributor

shadcn commented Jul 6, 2020

@arlina-espinoza Are we adding the max-age to all entity list builders or apps only? AFAIK we don't have a global max-age config.

@arlina-espinoza
Copy link
Contributor

@arshad Lets add it to the developer apps and team apps. Teams also have separate caching options but those should have been fixed in #419 already, and developers also have custom cache settings, but there isn't a list builder for them, so we should be good fixing these two.

shadcn added a commit to shadcn/apigee-edge-drupal that referenced this issue Jul 7, 2020
shadcn added a commit to shadcn/apigee-edge-drupal that referenced this issue Jul 7, 2020
shadcn added a commit to shadcn/apigee-edge-drupal that referenced this issue Jul 7, 2020
@arunz6161 arunz6161 added this to the 8.x-1.12 milestone Jul 7, 2020
shadcn added a commit to shadcn/apigee-edge-drupal that referenced this issue Jul 14, 2020
shadcn added a commit to shadcn/apigee-edge-drupal that referenced this issue Jul 16, 2020
arlina-espinoza pushed a commit that referenced this issue Jul 20, 2020
)

* [#430] Add configurable cache max age for edge entity list builder

* [#430] Switch to if else

* [#430] Remove dump call

* [#430] Add user cache contexts

* [#430] Remove unused class
shadcn added a commit to shadcn/apigee-edge-drupal that referenced this issue Jul 20, 2020
shadcn added a commit to shadcn/apigee-edge-drupal that referenced this issue Jul 21, 2020
arlina-espinoza added a commit that referenced this issue Sep 8, 2020
* [#432] Implement credential deletion and generation

* [#432] Add operations links and expiration date

* [#432] Add tests

* [#432] Implement routes for Team App

* [#430] Refactor to a trait

* [#432] Lock rules version

* [#432] Fix rules version

* [#430] Fix remove product event test

* [#432] Remove approve operation

* [#432] Update credential form to use most recently active one

* [#432] Update tests

* [#432] Fix phpcs notices

* [#432] Make add key form use modal

* [#432] Rename credential to api keys

* [#432] Fix conflicts

* [#432] Fix merge conflicts

* [#432] Update selector for dropbutton

* [#432] Revert php change

* [#432] Do not show warning for revoked credentials

* [#432] Skip api key routes for permission tests

* [#432] Implement api keys permissions

* [#432] Add tests for expired credentials warning

* [#432] Remove update access check

* [#432] Hide revoke credentials in collapsible section

* [#432] Fix tests

* [#432] Add api key routes permission tests

* [#432] Add add_api_key to permission matrix

* [#432] Update route for team test

* [#432] Prevent revoking/deleting the only active key

* [#432] Fix tests for new permission

* [#432] Use machine name

* [#432] Clean up developer and developerApp in tear down

* [#432] Add permission for editing API products

* [#432] Assign default permissions

* [#432] Update test permissions

* Fix credentials show/hide functionality issue when it gets revoked.

* Add revoke_api_key permission to authenticated users

* Remove add_keys from under content

* [#432] Add "edit_api_products" to authenticated user default permissions.

* [#432] Fix DeveloperAppApiKeysPermissionTest for integration and mock tests.

* [#432] Fix DeveloperAppApiKeyTest for integration and mock tests.

Co-authored-by: Minnur Yunusov <[email protected]>
Co-authored-by: Arlina Espinoza <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants