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

Feature 72 product in organization #103

Merged
merged 33 commits into from
Apr 4, 2024
Merged

Feature 72 product in organization #103

merged 33 commits into from
Apr 4, 2024

Conversation

berki0
Copy link
Contributor

@berki0 berki0 commented Apr 1, 2024

No description provided.

berki0 and others added 24 commits March 24, 2024 19:56
@berki0
[feature-70-BE-Expose-Organization-destruction] - changes after review
832f625
@berki0
[feature-72-BE-products-in-organization] CRUD
f90569e
@berki0
[feature-72-BE-products-in-organization] add deleting method
3281bb8
@berki0
[feature-72-BE-products-in-organization] add deleting method
b2af645
@ignatIgnatov
update product, entity fetcher
f6ff5c1
@berki0
[feature-72-BE-products-in-organization] add tests
4f6732e
@berki0
[feature-72-BE-products-in-organization] add tests
f42a518
@berki0
[feature-72-BE-products-in-organization] add tests
b108aae
@berki0
[feature-72-BE-products-in-organization] add tests
76b0db8
@berki0
[feature-72-BE-products-in-organization] add tests
3f9fc72
@berki0
[feature-72-BE-products-in-organization] add tests
283f2bb
@berki0
[feature-72-BE-products-in-organization] add tests
c7df993
@berki0
[feature-72-BE-products-in-organization] add tests
9d1a46e
@berki0
[feature-72-BE-products-in-organization] add tests
5842c84
@berki0
[feature-72-BE-products-in-organization] add tests
34484b6
@berki0
[feature-72-BE-products-in-organization] add tests
89c3c5a
@berki0
[feature-72-BE-products-in-organization] add tests
851ccf0
@berki0
[feature-72-BE-products-in-organization] add tests
2577835
@berki0
[feature-72-BE-products-in-organization] add tests
18f7185
@berki0
[feature-72-BE-products-in-organization] add tests
6025974
@ignatIgnatov
working create and update integration tests
a434678
@ignatIgnatov
refactoring
d47aede
@ignatIgnatov
working delete integration test
935639a
@berki0
[feature-72-BE-products-in-organization] add tests
0d9e089
@berki0 berki0 requested review from VladoKat, ignatIgnatov and Alekseeybg and removed request for VladoKat and ignatIgnatov April 1, 2024 05:47
VladoKat
VladoKat requested changes Apr 2, 2024
View reviewed changes
src/main/java/jewellery/inventory/model/resource/ResourceInProduct.java Outdated Show resolved Hide resolved
src/main/java/jewellery/inventory/service/ProductInOrganizationService.java Outdated Show resolved Hide resolved
src/main/java/jewellery/inventory/service/ProductInOrganizationService.java Outdated Show resolved Hide resolved
src/main/java/jewellery/inventory/service/ProductInOrganizationService.java Show resolved Hide resolved
src/test/java/jewellery/inventory/integration/ProductInOrganizationCrudIntegrationTest.java Outdated Show resolved Hide resolved
src/test/java/jewellery/inventory/integration/ProductInOrganizationCrudIntegrationTest.java
Copy link
Contributor

Choose a reason for hiding this comment

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

There seems to be some repeating code in the methods below. Can we think of a way to reduce the code duplication? Maybe some of the setups are very similar and can extracted?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should at least do the same section separation here as we did in src/test/java/jewellery/inventory/unit/service/ProductInOrganizationServiceTest.java

src/test/java/jewellery/inventory/unit/service/ProductInOrganizationServiceTest.java Outdated Show resolved Hide resolved
src/test/java/jewellery/inventory/unit/service/ProductInOrganizationServiceTest.java Show resolved Hide resolved
src/test/java/jewellery/inventory/helper/ResourceInOrganizationTestHelper.java Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

please, rearrange the code in each test to have 3 sections - preparation, action, assertions, with a single empty line between them

src/test/java/jewellery/inventory/unit/service/ProductServiceTest.java
verify(productRepository, times(1)).save(innerProduct);
verify(productRepository, times(2)).save(innerProduct);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this happening? From what I saw we haven't changed ProductService code?

@berki0 berki0 force-pushed the Feature-72-Product-in-Organization branch from 904f197 to 2063c21 Compare April 3, 2024 07:17
VladoKat
VladoKat requested changes Apr 3, 2024
View reviewed changes
src/main/java/jewellery/inventory/model/resource/ResourceInProduct.java
Copy link
Contributor

Choose a reason for hiding this comment

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

obsolete change?

src/test/java/jewellery/inventory/integration/ProductInOrganizationCrudIntegrationTest.java
Copy link
Contributor

Choose a reason for hiding this comment

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

We should at least do the same section separation here as we did in src/test/java/jewellery/inventory/unit/service/ProductInOrganizationServiceTest.java

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Apr 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@berki0 berki0 merged commit 3be7168 into test Apr 4, 2024
3 checks passed
@berki0 berki0 deleted the Feature-72-Product-in-Organization branch April 4, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VladoKat VladoKat VladoKat approved these changes

@Alekseeybg Alekseeybg Awaiting requested review from Alekseeybg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@berki0 @VladoKat @ignatIgnatov