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

Add new settings for default dataset & job sort orders #3369

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Buckminsterfullerene02
Copy link

@Buckminsterfullerene02 Buckminsterfullerene02 commented Dec 21, 2024

Proposed changes

As a daily user of Zowe Explorer, I have found it annoying that the default sort order for datasets and jobs is never the one I want. Therefore, the following two new settings are added:

  • zowe.ds.default.sort
  • zowe.jobs.default.sort

The new settings use the same defaults as is currently hardcoded for datasets and jobs.

The "(default)" added to the side of the sort order method is no longer hardcoded and is changed based on the settings.

Why do the setting have to be edited in settings.json? Because the alternatives (I thought of) are:

  • Add 4 new settings instead: dataset sort method, dataset sort direction, job sort method, job sort direction, as dropdowns. This seems too much for such a minor setting.
  • Have only 2 dropdowns as the above settings, but have the 8 and 10 permutations in each dataset and job dropdowns respectively. Also seems a bit excessive to me.

Another change that could be made, is to add sort order per dataset, and for jobs, as a persistence, like favourites. I actually started doing this as the first iteration of the feature, but realised the default settings would be easier to do at first, then try persistence, as it seems a bit more complex. Let me know what the thoughts on that are.

Release Notes

Milestone:

Changelog: Added two new settings, zowe.ds.default.sort and zowe.jobs.default.sort, that allow you to change the default sorting order of datasets and jobs when you open them.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Breaking change (a change that would cause existing functionality to not work as expected)
  • Documentation (Markdown, README updates)
  • Other (please specify above in "Proposed changes" section)

Checklist

General

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • All PR dependencies have been merged and published (if applicable)
  • A GIF or screenshot is included in the PR for visual changes
  • The pre-publish command has been executed:
    • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
    • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

  • There is coverage for the code that I have added
  • I have added new test cases and they are passing
  • I have manually tested the changes

Deployment

  • I have added developer documentation (if applicable)
  • Documentation should be added to Zowe Docs
    • If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
    • Otherwise, please check with the rest of the squad about any needed documentation before merging.
  • These changes may need ported to the appropriate branches (list here):

Further comments

This is my first contribution to this project. I have read as much of the contributor guidelines as I can, but there is a lot, so I'm sorry if I've forgotten or missed something. It also means that naturally I am still learning the codebase, so changes may be sub-par. Therefore, I have decided to open this as a draft PR with no test cases written yet. This isn't in the roadmap nor any issue has been raised, so I'd like to make sure the team is ok with this existing at all.

Lastly, I feel like my development iteration experience is too manual and that I must be missing something/doing something wrong. Currently, when I am making changes, I have pnpm watch:zowe-explorer script running. To build, I do pnpm install && pnpm run package. Then I go to extensions -> install from VSIX -> select zowe explorer. Then I must restart vscode (luckily it only takes a few seconds). When I want to look at debug logs, I need to manually open \.vscode\extensions\zowe.vscode-extension-for-zowe-3.1.0-SNAPSHOT\logs, which I also have to close (and not be in the folder in file explorer) for vscode to actually install the new VSIX file. What am I missing?

Added two new settings:
- zowe.ds.default.sort
- zowe.jobs.default.sort

They use the same defaults as is currently hardcoded.

The '(default)' is no longer hardcoded and is changed based on the settings.

TODO:
Tests need to be edited/written to reflect new feature/changes.
Perhaps include sort order persistence in the feature?

Signed-off-by: Buckminsterfullerene02 <[email protected]>
Signed-off-by: Buckminsterfullerene02 <[email protected]>
@adam-wolfe
Copy link
Contributor

Hi @Buckminsterfullerene02, for faster debugging, you can run the extension by pressing F5 and the logs can be viewed in real time using the Output tab in the VS Code terminal area and selecting Zowe Explorer from the drop down menu.

based off my current understanding of Jest, and not ran locally yet due to local set issues)

Signed-off-by: Buckminsterfullerene02 <[email protected]>
@Buckminsterfullerene02
Copy link
Author

Buckminsterfullerene02 commented Jan 5, 2025

Thank you so much Adam for the tip on the debugging, it's a life saver! Missed the info nugget in the docs.

Working on the Jest tests now, I do still have some problems with my local setup, perhaps with some kind of misconfiguration, where every test in the project fails or has unknown result, and the errors are all claiming to be syntax errors but are definitely not correct. For example:

 FAIL  packages/zowe-explorer/__tests__/__unit__/trees/uss/USSTree.unit.test.ts
  ● Test suite failed to run

    SyntaxError: F:\Github Projects\Work\zowe-explorer-vscode-fork\packages\zowe-explorer\__tests__\__unit__\trees\uss\USSTree.unit.test.ts: Unexpected token, expected "," (201:22)

      199 |     globalMocks.mockDefaultProfile.mockReturnValue(globalMocks.testProfile);
      200 |     globalMocks.getConfiguration.mockReturnValue({
    > 201 |         get: (_setting: string) => ["[test]: /u/aDir{directory}", "[test]: /u/myFile.txt{textFile}"],
          |                       ^
      202 |         update: jest.fn(() => {
      203 |             return {};
      204 |         }),

all the "failed" tests are for similar invalid reasons, which leads me to suspect Jest is somehow not configured to understand Typescript properly. But the tsconfig files seem to be correct, and clearly it must work for other people, so I really don't know what I am doing wrong here. Would appreciate some help because I'd like to work on some of the other issues (maybe a good first issue one to start off) too. If it would be any easier for a live chat, you know where to find me (on discord). Cheers!

@traeok
Copy link
Member

traeok commented Jan 7, 2025

Hi @Buckminsterfullerene02,

Are you trying to run a test in a specific package from the root of the repository? If you want to run tests for a specific file, you'll need to cd into the appropriate package and then run the test command, e.g.:

cd packages/zowe-explorer && npx jest USSTree.unit.test.ts

@Buckminsterfullerene02
Copy link
Author

Hi @Buckminsterfullerene02,

Are you trying to run a test in a specific package from the root of the repository? If you want to run tests for a specific file, you'll need to cd into the appropriate package and then run the test command, e.g.:

cd packages/zowe-explorer && npx jest USSTree.unit.test.ts

Thanks traeok, this works :-)
I was using the jest vscode extension that integrates the tests into panels and such, rather than just being on the commandline, as it had seemed to me that the monorepo was setup in a way that allows it to work, but I guess not. But I am more than happy to stick to the commandline.

But need to create mock settings for other failing tests in Profiles.unit.tests.ts and other unit tests. Fix circular dependency.

Signed-off-by: Buckminsterfullerene02 <[email protected]>
@zFernand0 zFernand0 self-requested a review January 8, 2025 20:17
@t1m0thyj t1m0thyj self-requested a review January 9, 2025 15:24
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Jan 9, 2025
@Buckminsterfullerene02
Copy link
Author

Buckminsterfullerene02 commented Jan 9, 2025

There are 2 failing tests in Profiles.unit.test.ts that I do not know how to fix:

  • should return the user credentials which seems to be failing even more making any changes to this test file, so I have no idea
  • should retrieve the secure properties of a profile which only started failing after I added this into the createGlobalMocks (which is required to fix the issue of many test suites failing due to the undefined property in SettingsConfig.getDirectValue):
    Object.defineProperty(SettingsConfig, "getDirectValue", {
        value: createGetConfigMock({
            "zowe.ds.default.sort": Sorting.DatasetSortOpts.Name,
            "zowe.jobs.default.sort": Sorting.JobSortOpts.Id,
        }),
        configurable: true,
    });

Any help would be greatly appreciated.

Buckminsterfullerene02 and others added 3 commits January 9, 2025 20:52
Signed-off-by: Buckminsterfullerene02 <[email protected]>
Signed-off-by: Buckminsterfullerene02 <[email protected]>
Which refreshes them with the new sort order setting.

Signed-off-by: Buckminsterfullerene02 <[email protected]>
@Buckminsterfullerene02
Copy link
Author

Buckminsterfullerene02 commented Jan 10, 2025

Todo:

  • Fix on configuration changed not updating for favourites - probably to do with the refreshing favourites tree, as the sort property is being updated fine
  • Fix the sorting button on the dataset session tree only sorting by name and ascending regardless of the sort settings it has
  • Add mocks for changing the sort order configurations on the fly
  • Add/edit unit tests for the onDidChangeConfiguration in DatasetTree.ts and JobTree.ts
  • Fix the 2 failing Profile.unit.test.ts tests
  • Fix the 3 failing tests in SettingsConfig.unit.test.ts
  • Fix the 2 failing tests in ZoweLocalStorage.unit.test.ts
  • Sort out adding these settings to the docs

Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Buckminsterfullerene02 <[email protected]>
Copy link

codecov bot commented Jan 12, 2025

Codecov Report

Attention: Patch coverage is 98.11321% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.20%. Comparing base (baade3a) to head (d75faab).

Files with missing lines Patch % Lines
...ges/zowe-explorer/src/trees/dataset/DatasetTree.ts 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3369      +/-   ##
==========================================
+ Coverage   93.18%   93.20%   +0.01%     
==========================================
  Files         120      120              
  Lines       12486    12535      +49     
  Branches     2715     2849     +134     
==========================================
+ Hits        11635    11683      +48     
- Misses        850      851       +1     
  Partials        1        1              

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

5 participants