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

Refactor/2318/migrate download button #2973

Merged
merged 15 commits into from Aug 11, 2022

Conversation

shaman-apprentice
Copy link
Contributor

@shaman-apprentice shaman-apprentice commented Aug 10, 2022

please not, that downloading and uploading of a map results in a loss of nodes (exact same behaviour as in main). As the download button will be removed in the future, we decided to not spent further time into this. But this is not related to the Angular migration for sure. #2704 might be related.

@shaman-apprentice shaman-apprentice marked this pull request as ready for review August 10, 2022 12:27
expect(mockedDownload).toHaveBeenCalled()
expect(mockedDownload.mock.calls[0][0]).toBe(mockedAccumulatedData["unifiedMapNode"])
expect(mockedDownload.mock.calls[0][1]).toBe(mockedAccumulatedData["unifiedFileMeta"])
expect(mockedDownload.mock.calls[0][3].length).toBe(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the FileSettings in the mock call? I.e., I would have exspected an assertion for the value of the 3rd parameter:

expect(mockedDownload.mock.calls[0][2]).toBe(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a plain getter from mocked state. I think mocking it usefully and checking against the mock doesn't provide any further insurance and is therefore not worth the effort

@knoffi
Copy link
Contributor

knoffi commented Aug 11, 2022

I loved the code. It was very smooth to read.

@sonarcloud
Copy link

sonarcloud bot commented Aug 11, 2022

[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Aug 11, 2022

[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

97.2% 97.2% Coverage
0.0% 0.0% Duplication

@knoffi knoffi self-requested a review August 11, 2022 14:33
Copy link
Contributor

@knoffi knoffi left a comment

Choose a reason for hiding this comment

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

LGTM!

@shaman-apprentice shaman-apprentice merged commit a203f5f into main Aug 11, 2022
@shaman-apprentice shaman-apprentice deleted the refactor/2318/migrate-download-button branch August 11, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants