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.

Sign up for GitHub

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

Jump to bottom

Refactoring and Clean Code of the DataFlow API #72

Merged
merged 25 commits into from Apr 14, 2022
Merged

Refactoring and Clean Code of the DataFlow API #72

merged 25 commits into from Apr 14, 2022

Conversation

floomm
Copy link
Contributor

@floomm floomm commented Apr 7, 2022 •
edited

Content of this PR

  1. Code smells were removed.
  2. Documentation in IDataFlowService.cs was added.
  3. Flooq database generates the uuid now by itself (when POSTing a new data flow). Consequently, the uuid is now nullable when POSTing a new data flow.
  4. The DataFlowController now adjusts the lastEdited field automatically when a DataFlow is overriden with PUT.

Related Issues

No related issues.

Definition of Done

  • Code
    • No more code needed
    • No known bugs
  • Clean Code
    • Comments have been created for new code
    • No unavoidable Code Smells
  • Testing
    • Existing Tests pass
  • SonarCloud Quality Gate passed

Definition of more than Done (optional)

  • Testing
    • New Integration Tests created
    • New Integration Tests passed

@floomm floomm added the api This issue is related to the API label Apr 7, 2022
@floomm floomm self-assigned this Apr 7, 2022
@floomm floomm requested a review from nikipuk April 7, 2022 07:15
@nikipuk
Copy link
Collaborator

nikipuk commented Apr 7, 2022

the code sells sound to me like it is not clearly defined, what is nullable or not

@sonarcloud
Copy link

sonarcloud bot commented Apr 7, 2022

[API] 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 2 Code Smells

97.1% 97.1% Coverage
0.0% 0.0% Duplication

DuplosFidibuss
DuplosFidibuss suggested changes Apr 8, 2022
View reviewed changes
Copy link
Contributor

@DuplosFidibuss DuplosFidibuss left a comment

Choose a reason for hiding this comment

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

Very good refactoring. Please have a look at the two comments.

src/api/Flooq.Api/Services/IDataFlowService.cs Outdated
Comment on lines 27 to 31
/// Sets a DataFlow entry according to the given EntityState.
/// For example if EntityState.Modified is used, the existing DataFlow entry will be overwritten by the new DataFlow.
/// </summary>
/// <param name="dataFlow">The new DataFlow.</param>
/// <param name="entityState">Defines the action of the new DataFlow entry.</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite clear what you mean here - is a new data flow created in any case? Otherwise the expression "The new DataFlow" would not be precise.

src/api/Flooq.Api/Services/IDataFlowService.cs Outdated
Comment on lines 34 to 36
/// <summary>
/// Saves all changes made in this context to the database.
/// </summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Which context?

@MrF3lix
Copy link
Contributor

MrF3lix commented Apr 11, 2022

@nikipuk will you do the necessary changes in the editor?

nikipuk
nikipuk reviewed Apr 13, 2022
View reviewed changes
Copy link
Collaborator

@nikipuk nikipuk left a comment

Choose a reason for hiding this comment

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

Do the Migrations have to stay in the project?

@nikipuk
Copy link
Collaborator

nikipuk commented Apr 13, 2022

@nikipuk will you do the necessary changes in the editor?

I did

@nikipuk nikipuk requested review from nikipuk and removed request for nikipuk April 14, 2022 06:29
@sonarcloud
Copy link

sonarcloud bot commented Apr 14, 2022

[Executor] 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 Apr 14, 2022

[API] 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 8 Code Smells

91.8% 91.8% Coverage
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Apr 14, 2022

[Editor] 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
0.0% 0.0% Duplication

@floomm floomm merged commit ca056bd into main Apr 14, 2022
@nikipuk nikipuk deleted the refactor/api/dataflow branch April 14, 2022 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikipuk nikipuk nikipuk left review comments

@DuplosFidibuss DuplosFidibuss DuplosFidibuss approved these changes

Assignees

@floomm floomm

Labels
api This issue is related to the API
Projects
Development V1
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@floomm @nikipuk @MrF3lix @DuplosFidibuss