feat: Changed the constants to variables under account/settings reopened
#1297
Conversation
and INTO-CPS-Association#1277) Moves the pipeline execution files into the stable codebase and prepares it to run multiple executions --------- Co-authored-by: Microchesst <marcusnj123@gmail.com> Co-authored-by: Marcus Jensen <79929209+Microchesst@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…amedeveloper/DTaaS into feature/distributed-demo
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feature/distributed-demo #1297 +/- ##
============================================================
+ Coverage 95.40% 95.72% +0.31%
============================================================
Files 98 103 +5
Lines 2744 2875 +131
Branches 440 480 +40
============================================================
+ Hits 2618 2752 +134
+ Misses 123 120 -3
Partials 3 3
🚀 New features to boost your workflow:
|
|
@atomicgamedeveloper The settings are completely independent of the OAuth user information coming from gitlab. It is ok to skip the handling of OAuth sessionstorage issue from this PR. |
There was a problem hiding this comment.
Pull Request Overview
This PR converts hardcoded GitLab constants into configurable settings that users can modify through a new settings form in the Account section. The implementation provides a Redux-based settings management system with persistent localStorage storage.
- Created a settings Redux slice with persistent localStorage integration
- Added a new settings form UI in the Account section for users to customize GitLab configurations
- Replaced direct constant usage with getter functions throughout the codebase to support dynamic configuration
Reviewed Changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/store/settings.slice.ts | New Redux slice managing GitLab settings with localStorage persistence |
| client/src/route/account/SettingsForm.tsx | New React component providing a form interface for modifying GitLab settings |
| client/src/model/backend/gitlab/digitalTwinConfig/settingsUtility.ts | New utility functions providing both hook and non-hook access to settings |
| client/src/store/store.ts | Updated to include settings slice and middleware for localStorage persistence |
| client/test/unit/store/settings.slice.test.ts | Unit tests for the settings slice functionality |
| client/test/unit/routes/SettingsForm.test.tsx | Unit tests for the settings form component |
| client/test/e2e/tests/Settings.test.ts | End-to-end tests for the settings functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@copilot review |
prasadtalasila
left a comment
prasadtalasila
left a comment
There was a problem hiding this comment.
@atomicgamedeveloper please see a partial review of the PR.
client/src/model/backend/gitlab/digitalTwinConfig/settingsUtility.ts
Outdated
Show resolved
Hide resolved
| ], | ||
| globalSetup: 'test/e2e/setup/global.setup.ts', | ||
| globalTeardown: 'test/e2e/setup/global-teardown.ts', | ||
| reportSlowTests: null, |
There was a problem hiding this comment.
How does this help?
There was a problem hiding this comment.
Some of our tests are slow so this gets rid of the warning.
prasadtalasila
left a comment
prasadtalasila
left a comment
There was a problem hiding this comment.
@atomicgamedeveloper Thanks for the updates. Please check the comments. In addition to them so general comments.
- Please update
classDiagramOfPreviewUtil.png - The LocalStorage of the browser has two JSON objects -
dtaas_settingsandsettings. Both contain the same JSON object. Is it possible to just use one, saysettings?
client/src/model/backend/gitlab/digitalTwinConfig/settingsUtility.ts
Outdated
Show resolved
Hide resolved
| expect(mockApi.createRepositoryFile).toHaveBeenCalledWith( | ||
| 2, | ||
| 'path/file', | ||
| 'master', |
There was a problem hiding this comment.
Can this branch name be flexible so that the changes made by Settings can potentially be tested? I guess mock handling the branch name can be changed to be more flexible.
- Changed imports from 'model/backend/gitlab/UtilityInterfaces' to 'model/backend/interfaces/utilityInterfaces' in multiple files. - Updated the usage of FileState and FileType enums to ensure consistency across the application. - Refactored file type handling in various components to use the new FileType enum. - Adjusted tests to reflect changes in file type handling and imports. - Cleaned up unused imports and improved code readability.
…se getBranchName in most tests
- Update documentation - Add missing coverage - Removed afterEach hooks that called jest.clearAllMocks() from multiple test files as mocks were being reset consistently - Ensured that jest.restoreAllMocks() is still called where necessary to reset mock implementations.
|
prasadtalasila
merged commit 6802dbd
into
INTO-CPS-Association:feature/distributed-demo
3 participants
Add configurable GitLab settings in user account section
Type of Change
Description
This PR addresses issue #1211 by converting hardcoded GitLab constants into settings that users can modify through a new settings form in the Account section. It is a reopened version of #1249 to finalize it with tests and new constants, like
commonLibraryName. Key changes include:These changes allow users to customize:
Testing
Impact
Additional Information
The settings form includes a reset to defaults option in case someone needs to revert their changes.
Checklist