[Issue #1449] Update removeJob command to refresh the profiles cache separately from refreshing a tree

[nickImbirev]

Proposed changes

Resolves the issue #1449 by changing semantics of the refreshAll util function. Now, we need to refresh the profiles cache independently from any tree provider.

Screen.Recording.2021-10-08.at.15.44.12.mov

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Updates to Documentation or Tests (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTOR GUIDANCE wiki
• PR title follows Conventional Commits Guidelines
• PR Description is included
• gif or screenshot is included if visual changes are made
• yarn workspace vscode-extension-for-zowe vscode:prepublish has been executed
• All checks have passed (DCO, Jenkins and Code Coverage)
• I have added unit test and it is passing
• I have added integration test and it is passing
• There is coverage for the code that I have added
• I have tested it manually and there are no regressions found
• I have added necessary documentation (if appropriate)
• Any PR dependencies have been merged and published (if appropriate)

[JillieBeanSim]

@nickImbirev I don't see the tree refreshing and removing the deleted job(s) after confirmation. I have to right click the session and select refresh for the deleted jobs to be removed from the tree.

[nickImbirev]

@nickImbirev I don't see the tree refreshing and removing the deleted job(s) after confirmation. I have to right click the session and select refresh for the deleted jobs to be removed from the tree.

Hey @JillieBeanSim ! Thank you for the review!

Yeah, I can see, that such behaviour is possible, I do not think, it is somehow related to my changes, it is just because sometimes refresh action become fulfilled faster, than something was deleted in the backend (in Jes in this particular case). That's why sometimes you have deleted jobs in the treeView, but they will go away after the addition refresh.

But I saw such a behaviour even with the previous implementation. I think, previous behaviour to refresh everything after deletion was slower, so backend could be updated with the higher probability.

If we want to fix it completely - we need to fix it in the Zowe SDK or even in the Zosmf backend part, because we are just consuming their APIs.

I asked @jellypuno to test it as well, just to make sure, this behaviour is not operating system dependent.

Does it make sense?

[jellypuno]

@JillieBeanSim @nickImbirev This works for me. The profilesCache are not reset. I didn't have to enter my credentials again.

[jellypuno]

lgtm!

[JillieBeanSim]

@jellypuno @nickImbirev this is what I am seeing testing this. Errors pop up if deleted when opened by clicking the job id link plus what I mentioned above. I will also test against main to see if any of it is pre-existing (I don't remember seeing this issue) or if it's a regression

[nickImbirev]

@jellypuno @nickImbirev this is what I am seeing testing this. Errors pop up if deleted when opened by clicking the job id link plus what I mentioned above. I will also test against main to see if any of it is pre-existing (I don't remember seeing this issue) or if it's a regression

This is quite interesting, I cannot reproduce such an error on my local environment. But I can see, that the refresh sometimes may return already deleted job (apparently, the deletion was not completed on Zosmf side for some reason).

I can investigate this more, but honestly, I do not know where to dig, because we are consuming Zowe SDK API directly in our refresh and delete commands... I think, we have no such a behaviour before, because we refreshed everything during the deletion, so it took time for user to click on the node after it and the changes were synced in Zosmf side.

@JillieBeanSim what do you think about the possible solution to refresh everything in job treeView (like it was before), but keeping user's unsaved credentials? In that case, we can say that in 99% of cases, removed jobs will not be part of the tree, but users will have to reopen their sessions again manually.

[JillieBeanSim]

@nickImbirev if you can refresh the whole tree and keep the credentials for the profiles without, that would be preferable to these regressions that I didn't see on master branch when retesting the scenario. That would fix the issue at hand and we could create a new issue to figure out how to refresh the tree without closing it at another time

[lauren-li]

what do you think about the possible solution to refresh everything in job treeView (like it was before), but keeping user's unsaved credentials?

@nickImbirev I know this question wasn't for me, but after trying out this PR, I agree with your proposed approach here, as well. This way, it is just an improvement over the current release, without introducing new problems for end-users. As @JillieBeanSim suggested, we can use a separate issue to continue researching how to properly update/display the tree upon job deletion.

[nickImbirev]

I will update the approach slightly to follow @JillieBeanSim and @lauren-li suggestions

[jellypuno]

Looks good to me! Thanks @nickImbirev

[JillieBeanSim]

LGTM! thanks @nickImbirev for the fix

I do see some broken integration tests but also on Master branch. Created issue for that
#1595

I also see some files show up in Source Control that deal with the new FTP unit tests that should be ignored. Created issue for that too
#1594