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 Create tab into Digital Twins page preview #1005

Conversation

VanessaScherma
Copy link
Contributor

@VanessaScherma VanessaScherma commented Oct 25, 2024

Title

Add Create tab into Digital Twins page preview

Type of Change

  • New feature
  • Bug fix
  • Documentation update
  • Refactoring
  • Security patch
  • UI/UX improvement

Description

PR with the aim of solving issue #1002.

When opening, the Create page shows the Editor and three files already placed in the sidebar:

  • description.md
  • README.md
  • .gitlab-ci.yml

The user can:

  • insert a new file via the ‘Add new file’ button (it is not possible to insert files with the same name, but the extension can be different)
  • delete a file (either inserted by him or one of the predefined ones, but it is not possible to delete .gitlab-ci.yml as it is necessary for the execution of the DT)
  • rename a file inserted by him

By clicking on the ‘Create’ button, the user can enter the name of the DT, which will correspond to the name of the sub-folder within his GitLab digitaltwins folder, and confirm the operation.
At the end of the operation, the Snackbar informs of success or failure.

By switching to one of the other tabs, the DT is already visible.

Testing

Unit and integration tests are currently in progress.

Impact

These changes add a new functionality, allowing the user to insert new digital twins.

Additional Information

None.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have added tests for all the new code and any changes made to
    existing code.
  • I have made corresponding changes to the documentation.

@prasadtalasila
Copy link
Contributor

@VanessaScherma Thanks for the PR. A very top-level review.

What works:

  1. Saving of skeletal DT in the gitlab repository

What does not work:

  1. The newly created DT does not show up in Manage and Execute pages
  2. The new DT is not shown even after restarting the client and also not after relogin

What are the suggested improvements:

  1. Have a field for name of DT
  2. Permit users to save DTs even if the files are empty

@VanessaScherma
Copy link
Contributor Author

@prasadtalasila

The DT created should be immediately visible within the page, as shown in these screenshots:
CreateDT
Snackbar
Manage
In the example shown, I created a digital twin called ‘testDT’, correctly created as informed by the Snackbar at the bottom of the page after confirming the save. As soon as I changed tabs to Manage, it was immediately visible.

Regarding suggestions, I can move the field for the DT name from the Dialog to above the Editor, so that it is always visible. For empty files, on the other hand, I had initially left this option but a problem had arisen with Editor Monaco. In fact, if the file is empty, the text shown on EditorTab cannot be updated, showing the contents of the previously selected file. This is probably due to the ‘read-only’ option I added to avoid writing without having selected a file first. I can look for some alternative solution if you think the possibility of creating empty files is more suitable.

@prasadtalasila
Copy link
Contributor

@VanessaScherma

In the example shown, I created a digital twin called ‘testDT’, correctly created as informed by the Snackbar at the bottom of the page after confirming the save. As soon as I changed tabs to Manage, it was immediately visible.

Strange. I've created two new DTs, namely "new-dt" and "new-digital-twin". But they don't show up in Manage and Execute tabs.

Regarding suggestions, I can move the field for the DT name from the Dialog to above the Editor, so that it is always visible.

Ok thanks

For empty files, on the other hand, I had initially left this option but a problem had arisen with Editor Monaco. In fact, if the file is empty, the text shown on EditorTab cannot be updated, showing the contents of the previously selected file. This is probably due to the ‘read-only’ option I added to avoid writing without having selected a file first. I can look for some alternative solution if you think the possibility of creating empty files is more suitable.

It is alright for now. Please focus on completing the PR.

client/src/preview/util/gitlabDigitalTwin.ts Outdated Show resolved Hide resolved
client/src/preview/util/gitlabDigitalTwin.ts Outdated Show resolved Hide resolved
client/src/preview/route/digitaltwins/editor/Editor.tsx Outdated Show resolved Hide resolved
client/src/preview/route/digitaltwins/editor/Editor.tsx Outdated Show resolved Hide resolved
client/src/preview/route/digitaltwins/editor/Sidebar.tsx Outdated Show resolved Hide resolved
Copy link

codeclimate bot commented Oct 28, 2024

Code Climate has analyzed commit 28c38a7 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

View more on Code Climate.

Copy link
Contributor

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

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

@VanessaScherma Thanks for the updates. Broader suggestions are:

  1. The Execute and Manage pages refer to default branch while the create page refers to the main branch. Please make the create page refer to the default branch as well.
  2. When the tab is changed, the execution status of the DTs is lost. If it is easily doable, please retain the execution status including the waiting loop icon. Otherwise, skip it for now.
  3. The execution fails for a newly created DT
  4. Suggested text for different tabs in DigitalTwinTabDataPreview.ts is:
Create: Create and save new digital twins. The new digital twins are saved in the linked gitlab repository. Remember to add valid `.gitlab-ci.yml` configuration as it is used for execution of digital twin.

Manage: Explore, edit and delete existing digital twins. The changes get saved in the linked gitlab repository.

Execute: Execute existing digital twins using CI/CD pipelines of the linked gitlab repository. Availability of gitlab runners is required for execution of digital twins.

In addition, please see the comments.

client/src/preview/components/asset/AssetBoard.tsx Outdated Show resolved Hide resolved
client/src/preview/util/gitlabDigitalTwin.ts Outdated Show resolved Hide resolved
client/src/preview/store/file.slice.ts Outdated Show resolved Hide resolved
@prasadtalasila
Copy link
Contributor

The correct dependency graph is:

DigitalTwin (create, reconfigure, execute) ---> GitlabInstance (files, commits, jobs etc)
         |                                                        ^
         |                                                        |
         |----> Files (create, update, delete) -------------------|

You can create an interface IFiles and then have both GitlabInstance and Files implement this interface.

@prasadtalasila
Copy link
Contributor

The current code structure requires addition of child pipeline in parent pipeline. Thus each new DT created must have an entry in top-level .gitlab-ci.yml. Otherwise, the execution of new DTs fails.

The solution is to have the create step has update the .gitlab-ci.yml at the root-level. Otherwise, the new DTs do not get executed.

@VanessaScherma
Copy link
Contributor Author

The correct dependency graph is:

DigitalTwin (create, reconfigure, execute) ---> GitlabInstance (files, commits, jobs etc)
         |                                                        ^
         |                                                        |
         |----> Files (create, update, delete) -------------------|

You can create an interface IFiles and then have both GitlabInstance and Files implement this interface.

This is how the code was refactored:
The methods concerning files, previously placed in the DigitalTwin class, have now been moved to a FileHandler class. This class implements the IFiles interface. Each DigitalTwin has a FileHandler among its attributes, and initialises it in the constructor by passing it the same GitlabInstance associated with the DigitalTwin, which is used to call the APIs.

The only reference to files still present in DigitalTwin are the attributes descriptionFiles, configFiles, lifecycleFiles, since they are the file names closely related to the DT.

@VanessaScherma
Copy link
Contributor Author

@prasadtalasila
The handling of the .gitlab-ci.yml file is resolved. When a new DT is created, a trigger_DTName section is added to the .gitlab-ci.yml file in the parent pipeline. Similarly, this section is removed when deleting the corresponding DT from the Manage tab.

@prasadtalasila
Copy link
Contributor

@VanessaScherma please rebase the PR

@prasadtalasila
Copy link
Contributor

@VanessaScherma The addition of new DT adds entries in top-level .gitlab-ci.yml but the format is incorrect. As a result, an execution request on new DT triggers the following error.

image

The matching lines are:

image

The create request is adding extra tabs at the beginning of new lines. Could this be the error?

@prasadtalasila
Copy link
Contributor

another consequence is that the execution of existing DTs fail as well due to syntax error in top-level .gitlab-ci.yml.

Copy link
Contributor

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

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

@VanessaScherma Thanks for the updates. Please see the comments.

client/src/preview/util/init.ts Outdated Show resolved Hide resolved
client/src/preview/util/init.ts Show resolved Hide resolved
getDescriptionFiles(): Promise<string[]>;
getConfigFiles(): Promise<string[]>;
getLifecycleFiles(): Promise<string[]>;
appendTriggerToPipeline(): Promise<string>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move the trigger .gitlab-ci.yml functions to DTFiles class.

client/src/preview/util/ifile.ts Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

please merge this file into newly proposed DTFiles.ts


export interface IFile {
getFileContent(fileName: string, DTName: string): Promise<string>;
updateFileContent(fileName: string, fileContent: string): Promise<void>;
Copy link
Contributor

Choose a reason for hiding this comment

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

getFile() and updateFile() are consistent with createFile()

client/src/preview/util/digitalTwin.ts Show resolved Hide resolved
client/src/preview/util/digitalTwin.ts Outdated Show resolved Hide resolved
client/src/preview/util/digitalTwin.ts Outdated Show resolved Hide resolved
@prasadtalasila prasadtalasila added this to the Release v0.7.0 milestone Nov 11, 2024
@prasadtalasila
Copy link
Contributor

@VanessaScherma Apparently the tests inside and outside of test/preview folder are interacting. I've deleted the test/preview directory and ran yarn test:unit and yarn test:int. Please see the results in the log file.
test-log.log

@VanessaScherma
Copy link
Contributor Author

@prasadtalasila

I made a small modification to the jest.setup.ts of the integration code. Now, if the REACT_APP_AUTH_AUTHORITY is not found, it is inserted. The tests are successfully executed.

However, the 'Run client unit and integration tests' job fails with the yarn test:int command for low code coverage, as my files are covered by the yarn test:preview:int command. Is there a way to count the coverage using the two commands together?

client/package.json Outdated Show resolved Hide resolved
@prasadtalasila prasadtalasila merged commit 3264921 into INTO-CPS-Association:feature/distributed-demo Nov 14, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants