[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) #1207

AronPerez · 2025-01-03T16:45:19Z

PR Description

This PR adds Chai unit tests

Tasks

- A description of the PR has been provided, and a diagram included if it is a new feature.
- Formatter has been run
- CHANGELOG comment has been added
- Labels have been assigned to the pr
- A reviwer has been added
- A user has been assigned to work on the pr
- If new feature a unit test has been added

Summary by Sourcery

Tests:

Add unit tests for the mapped collection endpoint browse functionality.

sourcery-ai

Hey @AronPerez - I've reviewed your changes - here's some feedback:

Overall Comments:

The path handling change in transfer-ui-manager.js (removal of endsWith check) appears to be a behavioral change. Please clarify if this is intentional and how it affects the directory path handling.

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

AronPerez · 2025-01-07T22:24:05Z

Atrifacts

Unit Test validation

npm run test

t-ramz

Added a few comments/questions, and one concern.

web/static/components/transfer/transfer-ui-manager.js

web/test/setup.js

web/test/components/transfer/transfer-endpoint-manager.test.js

web/test/components/transfer/transfer-ui-manager.test.js

web/package.json.in

web/static/components/transfer/transfer-endpoint-manager.js

web/test/components/transfer/transfer-ui-manager.test.js

JoshuaSBrown

A positive contribution, adding first set of unit tests to the Frontend. I don't have any items that should be blocking for this PR if passing the CI. It would probably be a good idea to continute expand on the unit testing with some integration testing. One of the difficult parts of the current design is the different components are kind of tightly coupled. Perhaps it will make sense to put the api call pieces directly in some of the classes that are closely related at some point.

AronPerez · 2025-01-14T18:47:15Z

Artifacts

`npm run test` in `web/`

Screen.Recording.2025-01-14.at.1.27.00.PM.mov

`npm install` in `web/`

AronPerez · 2025-01-14T18:49:49Z

@sourcery-ai review

sourcery-ai · 2025-01-14T18:50:46Z

Reviewer's Guide by Sourcery

This PR adds unit tests for the Mapped Collection Endpoint Browse feature. It introduces Chai unit tests for the transfer-dialog-controller, transfer-endpoint-manager, and transfer-ui-manager components. Additionally, it updates the prettier dependency to version 3.4.2 and includes several new testing-related files.

Class diagram for transfer components under test

classDiagram
    class TransferDialogController {
        +handleDialog()
    }
    class TransferEndpointManager {
        +manageEndpoints()
    }
    class TransferUIManager {
        +manageUI()
    }

    TransferDialogController --> TransferUIManager
    TransferEndpointManager --> TransferUIManager

    note for TransferDialogController "Unit tests added"
    note for TransferEndpointManager "Unit tests added"
    note for TransferUIManager "Unit tests added"

File-Level Changes

Change	Details	Files
Added Chai unit tests for various transfer components.	Created new test files for each of the components. Implemented unit tests using Chai and Sinon to cover core functionality. Mocked necessary dependencies and UI elements for isolated testing scenarios. Added a setup file to configure the testing environment, including JSDOM, jQuery, and error handling. Created fixtures to mock services and jQuery components for consistent test setup. Added a Mocha configuration file to define test settings and include the setup file. Updated the Javascript Format workflow to use Prettier 3.4.2 for code formatting consistency.	`web/test/components/transfer/transfer-dialog-controller.test.js` `web/test/components/transfer/transfer-endpoint-manager.test.js` `web/test/components/transfer/transfer-ui-manager.test.js`
Updated npm dependencies and added new files for testing setup.	Updated the `package.json.in` file to include necessary testing dependencies. Created a `setup.js` file to configure the testing environment. Created `transfer-fixtures.js` to provide mock data and services for testing. Added a `.mocharc.cjs` file to configure Mocha test runner options. Updated the `javascript-format.yml` workflow file to install Prettier 3.4.2.	`web/package.json.in` `web/test/setup.js` `web/test/fixtures/transfer-fixtures.js` `web/.mocharc.cjs`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time. You can also use
this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @AronPerez - I've reviewed your changes - here's some feedback:

Overall Comments:

The PR includes empty test files (transfer-dialog-controller.test.js, transfer-endpoint-manager.test.js, transfer-ui-manager.test.js). Please implement the actual test cases before requesting review.

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

JoshuaSBrown · 2025-01-14T19:29:25Z

.github/workflows/javascript-format.yml

@@ -18,7 +18,7 @@ jobs:
      # Step 3: Install Prettier and ESLint globally
      - name: Install Prettier
        run: |
-          npm install -g prettier


Good catch.

t-ramz

LGTM

…)" This reverts commit 22545fa.

This reverts commit 0a56039. Revert "Revert "[DLT-1110] Upgrade node v23 -> v18, better jump"" This reverts commit b57bd27. Revert "Revert "[DLT-1110] Upgrade node v14 -> v23"" This reverts commit 6e3cc52 Revert "[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) (#1207)" This reverts commit 22545fa. Revert "[DLT-1110] Config" This reverts commit 82715f0. Revert "[DLT-1110] Upgrade node v14 -> v23" This reverts commit fb8b93d. Revert "[DLT-1110] Upgrade node v23 -> v18, better jump" This reverts commit 9d66847. Revert "[DLT-1110] Correct ver .sh file" This reverts commit 5c78c59. [DLT-1110] Correct ver .sh file [DLT-1110] Upgrade node v23 -> v18, better jump [DLT-1110] Upgrade node v14 -> v23 [DLT-1110] Config [DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) (#1207) * [DLT-1110] Add tests * [DLT-1110] Update chai with mocha, attempt usage of mock-import * [DLT-1110] Add mocha config, correct prettier * [DLT-1110] Update tests, correct logic * [DLT-1110] Update tests, add fixtures, update setup, lock in packages * [DLT-1110] Update test [DLT-1110] Update style [DLT-1110] Update if statements [DLT-1110] Dependency injection [DLT-1110] Fix update bug [DLT-1110] Pull out template HTML [DLT-1110] Correct HTML escape, francy tree init [DLT-1110] Add commnets to logic [DLT-1110] Update model, make logic private, remove controller form transfer request when update or get [DLT-1110] Split branches [DLT-1110] Update files with prettier [DLT-1110] Add controller unit tests [DLT-1110] Remove debug [DLT-1110] Correct import statements, endpoint list [DLT-1110] Refactor out into MVC [DLT-1110] Add debug [DLT-1110] Functioning modal, needs refactoring [DLT-1110] Refactor dlg start transfer using OOO programming. Should be MVC cased on what I'm seeing but we'll see

…/4) (#1207)"" This reverts commit ddfee85.

* [DLT-1110] Update style [DLT-1110] Update if statements [DLT-1110] Dependency injection [DLT-1110] Fix update bug [DLT-1110] Pull out template HTML [DLT-1110] Correct HTML escape, francy tree init [DLT-1110] Add commnets to logic [DLT-1110] Update model, make logic private, remove controller form transfer request when update or get [DLT-1110] Split branches [DLT-1110] Update files with prettier [DLT-1110] Add controller unit tests [DLT-1110] Remove debug [DLT-1110] Correct import statements, endpoint list [DLT-1110] Refactor out into MVC [DLT-1110] Add debug [DLT-1110] Functioning modal, needs refactoring [DLT-1110] Refactor dlg start transfer using OOO programming. Should be MVC cased on what I'm seeing but we'll see * [DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) (#1207) * [DLT-1110] Add tests * [DLT-1110] Update chai with mocha, attempt usage of mock-import * [DLT-1110] Add mocha config, correct prettier * [DLT-1110] Update tests, correct logic * [DLT-1110] Update tests, add fixtures, update setup, lock in packages * [DLT-1110] Update test * [DLT-1110] Config * [DLT-1110] Upgrade node v14 -> v23 * [DLT-1110] Upgrade node v23 -> v18, better jump * [DLT-1110] Correct ver .sh file * Revert "[DLT-1110] Correct ver .sh file" This reverts commit 5c78c59. * Revert "[DLT-1110] Upgrade node v23 -> v18, better jump" This reverts commit 9d66847. * Revert "[DLT-1110] Upgrade node v14 -> v23" This reverts commit fb8b93d. * Revert "[DLT-1110] Config" This reverts commit 82715f0. * Revert "[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) (#1207)" This reverts commit 22545fa.

* [DLT-1110] Add tests * [DLT-1110] Update chai with mocha, attempt usage of mock-import * [DLT-1110] Add mocha config, correct prettier * [DLT-1110] Update tests, correct logic * [DLT-1110] Update tests, add fixtures, update setup, lock in packages * [DLT-1110] Update test

AronPerez force-pushed the feat/DLT-1110/mapped-collection-endpoint-browse branch from d0215b7 to db9280d Compare January 3, 2025 16:48

AronPerez force-pushed the feat/DLT-1110/mapped-collection-endpoint-browse-tests branch from 3ed0070 to 500ba5b Compare January 3, 2025 16:49

AronPerez changed the title ~~[DLT-1110] Mapped Collection Endpoint Browse Tests~~ [DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) Jan 3, 2025

AronPerez force-pushed the feat/DLT-1110/mapped-collection-endpoint-browse-tests branch from c8672c0 to 39b451c Compare January 6, 2025 20:39

AronPerez marked this pull request as ready for review January 7, 2025 21:44

sourcery-ai bot reviewed Jan 7, 2025

View reviewed changes

t-ramz reviewed Jan 8, 2025

View reviewed changes

AronPerez added the Component: CI label Jan 8, 2025

JoshuaSBrown reviewed Jan 9, 2025

View reviewed changes

web/package.json.in Show resolved Hide resolved

JoshuaSBrown reviewed Jan 9, 2025

View reviewed changes

web/static/components/transfer/transfer-endpoint-manager.js Outdated Show resolved Hide resolved

JoshuaSBrown reviewed Jan 9, 2025

View reviewed changes

web/test/components/transfer/transfer-ui-manager.test.js Outdated Show resolved Hide resolved

JoshuaSBrown approved these changes Jan 9, 2025

View reviewed changes

AronPerez force-pushed the feat/DLT-1110/mapped-collection-endpoint-browse branch 3 times, most recently from 939f7f9 to c99d09d Compare January 10, 2025 20:17

AronPerez force-pushed the feat/DLT-1110/mapped-collection-endpoint-browse-tests branch 4 times, most recently from a23cc88 to 5dc8181 Compare January 14, 2025 18:43

AronPerez force-pushed the feat/DLT-1110/mapped-collection-endpoint-browse branch from dbe7432 to 3679a4f Compare January 14, 2025 18:46

AronPerez and others added 6 commits January 14, 2025 13:46

[DLT-1110] Add tests

36c8040

[DLT-1110] Update chai with mocha, attempt usage of mock-import

0203abb

[DLT-1110] Add mocha config, correct prettier

48cfaf1

[DLT-1110] Update tests, correct logic

23e2cf4

[DLT-1110] Update tests, add fixtures, update setup, lock in packages

3763ad2

[DLT-1110] Update test

f939461

AronPerez force-pushed the feat/DLT-1110/mapped-collection-endpoint-browse-tests branch from 5dc8181 to f939461 Compare January 14, 2025 18:46

ORNL deleted a comment from sourcery-ai bot Jan 14, 2025

sourcery-ai bot reviewed Jan 14, 2025

View reviewed changes

JoshuaSBrown reviewed Jan 14, 2025

View reviewed changes

t-ramz approved these changes Jan 14, 2025

View reviewed changes

AronPerez merged commit 22545fa into feat/DLT-1110/mapped-collection-endpoint-browse Jan 14, 2025
7 of 8 checks passed

AronPerez added this to the DataFed Mapped Collection Consent Support milestone Jan 22, 2025

AronPerez linked an issue Jan 22, 2025 that may be closed by this pull request

[Web, Globus] - Feature support listing collection contents from mapped collections from web ui #1110

Open

6 tasks

AronPerez removed a link to an issue Jan 22, 2025

[Web, Globus] - Feature support listing collection contents from mapped collections from web ui #1110

Open

6 tasks

AronPerez added a commit that referenced this pull request Jan 22, 2025

Revert "[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) (#1207…

ddfee85

…)" This reverts commit 22545fa.

AronPerez added a commit that referenced this pull request Jan 22, 2025

Revert "Revert "[DLT-1110] Mapped Collection Endpoint Browse Tests (2…

f75826d

…/4) (#1207)"" This reverts commit ddfee85.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) #1207

[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) #1207

AronPerez commented Jan 3, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot left a comment

AronPerez commented Jan 7, 2025

t-ramz left a comment

JoshuaSBrown left a comment

AronPerez commented Jan 14, 2025

AronPerez commented Jan 14, 2025

sourcery-ai bot commented Jan 14, 2025

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

JoshuaSBrown Jan 14, 2025

t-ramz left a comment

[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) #1207

[DLT-1110] Mapped Collection Endpoint Browse Tests (2/4) #1207

Conversation

AronPerez commented Jan 3, 2025 • edited by sourcery-ai bot Loading

PR Description

Tasks

Summary by Sourcery

sourcery-ai bot left a comment

Choose a reason for hiding this comment

AronPerez commented Jan 7, 2025

Atrifacts

Unit Test validation

t-ramz left a comment

Choose a reason for hiding this comment

JoshuaSBrown left a comment

Choose a reason for hiding this comment

AronPerez commented Jan 14, 2025

Artifacts

npm run test in web/

npm install in web/

AronPerez commented Jan 14, 2025

sourcery-ai bot commented Jan 14, 2025

Reviewer's Guide by Sourcery

Class diagram for transfer components under test

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

Choose a reason for hiding this comment

JoshuaSBrown Jan 14, 2025

Choose a reason for hiding this comment

t-ramz left a comment

Choose a reason for hiding this comment

AronPerez commented Jan 3, 2025 •

edited by sourcery-ai bot

Loading

`npm run test` in `web/`

`npm install` in `web/`