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

Functional behaviour corrections #186

Merged
merged 14 commits into from
Jan 17, 2025
Merged

Functional behaviour corrections #186

merged 14 commits into from
Jan 17, 2025

Conversation

chinmdas
Copy link
Contributor

@chinmdas chinmdas commented Dec 23, 2024

What It Does
As part of this PR, we have fixed following scenarios:

  1. Manage Profile option missing in CICS Explorer when we right click on available CICS Profile.
  2. If a config file is available, CICS Explorer currently shows Edit Team Configuration File option only, where as ZE shows Create a New Team Configuration File option as well.
  3. In CICS Explorer, currently we do not have option to create Project level Team Configuration file, i.e Project: in the current working directory.

How to Test
Tested against Zowe Explorer

Review Checklist
I certify that I have:

Additional Comments

Signed-off-by: Chinmay Das <[email protected]>
Signed-off-by: Chinmay Das <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2024

Codecov Report

Attention: Patch coverage is 11.42857% with 124 lines in your changes missing coverage. Please review.

Project coverage is 28.23%. Comparing base (eda5d41) to head (dc1e000).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/vsce/src/trees/CICSTree.ts 11.67% 121 Missing ⚠️
packages/vsce/src/commands/manageSessionCommand.ts 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #186      +/-   ##
==========================================
- Coverage   28.47%   28.23%   -0.25%     
==========================================
  Files         148      146       -2     
  Lines        4958     4990      +32     
  Branches      861      869       +8     
==========================================
- Hits         1412     1409       -3     
- Misses       3546     3581      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chinmdas
Copy link
Contributor Author

This PR fixes #179

@chinmdas
Copy link
Contributor Author

chinmdas commented Dec 23, 2024

I have recorded screen to show you guys the functional testing of CICS Extension.
I have started covering the scenarios with following chronological order:

  1. Started with NO configuration file (Local and Global both)
  2. Creation success + Edit Success
  3. Edited the config file to remove all the profile. Blank profile.
  4. No errors --> Creation success + Edit Success
  5. Replaced configuration file with proper connection details
  6. created 2 connection. 1 from Global config file + 1 from Local config file.
  7. Right click -> Manage Profile options available
  8. on click of Manage Profile -> Update / Hide / Delete
  9. Hide -> works
  10. Update / Delete -> open global config file when global config is edited.
  11. Update / Delete -> open local config file when local config is edited.
Screen.Recording.2024-12-24.at.10.01.10.AM.mov

@chinmdas chinmdas linked an issue Dec 24, 2024 that may be closed by this pull request
@davenice davenice mentioned this pull request Jan 6, 2025
4 tasks
Copy link
Contributor

@AndrewTwydell AndrewTwydell left a comment

Choose a reason for hiding this comment

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

Some comments to address, and some indentation to correct in the comments. Package.json also needs looking at - presumably you've changed the delete/update commands to a manage command, but hard to tell currently.

I also notice a bug where profiles that are already in the tree are not removed from the list of profiles to pick, like the extension currently does.

Screen.Recording.2025-01-07.at.12.16.12.PM.mov

Signed-off-by: Chinmay Das <[email protected]>
@chinmdas
Copy link
Contributor Author

chinmdas commented Jan 9, 2025

Some comments to address, and some indentation to correct in the comments. Package.json also needs looking at - presumably you've changed the delete/update commands to a manage command, but hard to tell currently.

I also notice a bug where profiles that are already in the tree are not removed from the list of profiles to pick, like the extension currently does.

Screen.Recording.2025-01-07.at.12.16.12.PM.mov

Thanks for reporting this @AndrewTwydell . I have fixed this...!!!

@chinmdas chinmdas requested a review from AndrewTwydell January 9, 2025 13:48
@davenice
Copy link
Contributor

I've have tested this and it seems good to me.

I've got a project config file and a global config file.

I've tried hiding connections from both.
After I hide, they appear correctly on the + menu
I've tried edit and delete on both and they open the correct files.

I haven't tried more complex things like having the same name connection in both types of file or similar.

Functionally this is taking us in a good direction.

@davenice
Copy link
Contributor

@AndrewTwydell could you check you're happy with the changes you requested. And if we get a review from @zFernand0 or someone in the Zowe Explorer team I think we're good to put this in later today. (We've got another icon change incoming so perhaps we batch them up)

@chinmdas chinmdas changed the base branch from main to sdk-unit-mocking January 15, 2025 08:11
@chinmdas chinmdas changed the base branch from sdk-unit-mocking to main January 15, 2025 08:11
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

I was about to approve the PR, but I merged another one that created some not-so-pretty conflicts for extension.ts 😢
I'm terribly sorry about that. 😢

@chinmdas
Copy link
Contributor Author

I was about to approve the PR, but I merged another one that created some not-so-pretty conflicts for extension.ts 😢 I'm terribly sorry about that. 😢

Thanks for taking out time to review this @zFernand0 . I'll resolve conflicts and keep it merge ready.

@chinmdas chinmdas requested a review from zFernand0 January 16, 2025 13:57
Copy link
Contributor

@davenice davenice left a comment

Choose a reason for hiding this comment

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

I have tested this as before - project + global team configuration files - all seems good.

I've made a couple of suggestions none of which are deal breakers.

Copy link
Contributor

@AndrewTwydell AndrewTwydell left a comment

Choose a reason for hiding this comment

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

Yeah few spacing changes but one of the next things we can merge is adding prettier which will format all the files correctly so no biggy.

@chinmdas
Copy link
Contributor Author

Hi @zFernand0 , please help us get this merged...!!!

@chinmdas chinmdas force-pushed the functional-ui-changes branch from 354d06d to 558dc45 Compare January 16, 2025 16:40
@chinmdas
Copy link
Contributor Author

Hi @zFernand0 , requesting your review on this, and may be a squash & merge if you find everything okayish.

Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋

Thanks @chinmdas

@zFernand0 zFernand0 merged commit e3d7757 into main Jan 17, 2025
19 checks passed
@zFernand0 zFernand0 deleted the functional-ui-changes branch January 17, 2025 15:30
@zFernand0 zFernand0 added the release-minor Indicates a minor feature has been added label Jan 17, 2025
Copy link

Release succeeded for the main branch. 🎉

The following packages have been published:

Powered by Octorelease 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-minor Indicates a minor feature has been added released
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Correct profile management functions in the CICS extension to match ZE
5 participants