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

Removed asset still shows up in asset list #3163

Closed
duvld opened this issue Apr 29, 2021 · 0 comments · Fixed by #3288
Closed

Removed asset still shows up in asset list #3163

duvld opened this issue Apr 29, 2021 · 0 comments · Fixed by #3288
Assignees
Labels

Comments

@duvld
Copy link
Member

duvld commented Apr 29, 2021

Description

Discovered while working on #3151 and side effect of #2984.

When we perform a self permission removal, the "removed" asset can still show up in the asset list (until user does a refresh).

Bug only happens if the non-owned asset is the last asset user has in their asset list.

Steps to Reproduce

Scenario 1:

  1. With Add non-owner self removal to form landing #3151, remove your own permissions via the menu action in the form landing page
  2. Get redirected to asset list

Scenario 2:

  1. Navigate to non-owned asset you want to remove
  2. Go back to asset list yourself
  3. Remove own permissions from asset list menu action

Expected behavior

"Removed" asset is gone from asset list

Actual behavior

"Removed" asset still exists in the list and clicking it yields an infinite loading screen

Additional details

This one goes pretty deep. In scenario 2, if the user does not navigate to the non-owned asset they want to remove and removes their own permissions from the list menu action, (i.e., fresh loading from asset list then imminently self remove permission) then the asset is gone from the list 👍

The reason for this bug is to do with search store states, and how they can be manipulated. The search store is populated (specifically in four lists: defaultQueryResultsList, searchResultsList, defaultQueryCategorizedResultsLists and searchResultsCategorizedResultsLists) when we navigate to a specific asset.

Regardless if we own the asset or not, when the user deletes their asset and is redirected back to the asset list the deleted asset is not removed entirely from the search store*

The difference between removed assets still in the search store is:

@@ -267,8 +266,7 @@
                 "children": {
                     "count": 0
                 },
-                "tags": [],
-                "deleted": "true"
+                "tags": []
             }
         ],
         "Draft": [],

This was never a problem for owned assets because deleted: true is appended to the asset in the search store state and the asset list ignores it**

Obviously the older form component code wasn't expecting you to "delete" an unowned asset so somehow deleted: true does not show up and in the search store state and the removed asset is displayed in the asset list.

There's a few ways we can go about hack fixing this:

  1. In rebuildCategorizedAsset, we can remove the check for sourceReuslts.length being 0, effectively removing every asset from the four lists and bypass the deleted: true. This results in a blank asset list with no table
  2. Stitch deleted: true to the lone leftover asset in the search store state if it was a self permission removal

Either way this is low priority as it can be negated by a refresh, and this doesn't result in any security issues with the search store.


*
This is because when we remove an asset, it calls removeAsset in searches.es6 and by the time we reach rebuildCategorizedAsset in removeAsset the function is ignored due to the guard for sourceReuslts.length being 0 as we have just called removeAssetFromList on the two search lists (defaultQueryResultsList and searchResultsList), thus leaving the removed asset intact in the search state's defaultQueryCategorizedResultsLists and searchResultsCategorizedResultsLists.

**
I can't quite find the exact reason for this, but I know it's related to the call to actions.resources.updateAsset when clicking a button in projectSettings.es6. I've made an issue about it a bit ago because I felt it was strange that we were giving "successfully updated" notifications before anything happens. Turns out this updating is the reason why deleted: true shows up for removed owned assets. Without this call prior to deletion the owned asset will show up in the asset list--it feels like doing this syncs up the asset in the search store. I'd love to get to the bottom of this but I'm definitely stuck here now. We also cannot do this call for non-owned assets (unless they have manage_asset) due to permissions. If the non-owned asset is not the last asset, none of the above matters because rebuildCategorizedAsset is able to filter out removed assets due to other assets existing (guard does not get skipped).

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 a pull request may close this issue.

1 participant