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 Remote Products to Remote Projects #90

Merged
merged 8 commits into from May 21, 2020
Merged

Conversation

kaladay
Copy link
Contributor

@kaladay kaladay commented May 19, 2020

This includes fixing the URLs.
Remote Projects should not be under the Product URL.

A new controller is brought in for providing /projects/remote/by-product/{productId}.

Fetching all remote projects was moved out of the /products URL and into the /remote-project-manager URL as follows:
- /remote-project-manager/{remoteProjectManagerId}/remote-projects
- /remote-project-manager/{remoteProjectManagerId}/remote-projects/{scopeId}

Resolves #85

Always associate the product received by remote products such as LSSS.
This requires redesigning the Service and UI to associate with a given product.
Whereas before, the product was only associated on push, the product is now associated on create/edit.
This includes fixing the URL.
Remote Projects should not be under the Product URL.

A new controller is brought in for providing `/projects/remote/by-product/{productId}`.

Fetching all remote projects was moved out of the `/products` URL and into the `/remote-project-manager` URL as follows:
- `/remote-project-manager/{remoteProjectManagerId}/remote-projects`
- `/remote-project-manager/{remoteProjectManagerId}/remote-projects/{scopeId}`
@kaladay kaladay requested a review from rladdusaw May 19, 2020 15:52
@coveralls
Copy link

coveralls commented May 19, 2020

Pull Request Test Coverage Report for Build 370

  • 205 of 213 (96.24%) changed or added relevant lines in 20 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.2%) to 95.015%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/edu/tamu/app/cache/service/RemoteProjectsScheduledCacheService.java 32 33 96.97%
src/main/java/edu/tamu/app/controller/ProductController.java 39 40 97.5%
src/main/java/edu/tamu/app/ProjectInitialization.java 0 2 0.0%
src/main/java/edu/tamu/app/model/InternalRequest.java 3 5 60.0%
src/main/java/edu/tamu/app/model/InternalStats.java 5 7 71.43%
Files with Coverage Reduction New Missed Lines %
src/main/java/edu/tamu/app/controller/ProductController.java 1 96.43%
src/main/java/edu/tamu/app/model/InternalStats.java 1 63.64%
Totals Coverage Status
Change from base Build 355: -0.2%
Covered Lines: 1277
Relevant Lines: 1344

💛 - Coveralls

@kaladay kaladay marked this pull request as ready for review May 19, 2020 17:03
@ghost
Copy link

ghost commented May 19, 2020

@rladdusaw changes to the existing API would be breaking and not within scope of the issue. These endpoints are being consumed by the project board Michael created. I suggest we produce a specification to decide API naming and then create issues for the changes.

/projects/remote -> /remote-projects did not fall in the purview of refactoring project to product.

@rladdusaw
Copy link
Contributor

@rladdusaw changes to the existing API would be breaking and not within scope of the issue.

@wwelling
We've broken the API completely already with the refactor. This is the perfect time to fix some paths that were relics of the old controller design.

This needs further discussion.
@kaladay kaladay requested a review from rladdusaw May 20, 2020 14:59
@kaladay kaladay merged commit 3dda57e into sprint5-staging May 21, 2020
@kaladay kaladay deleted the sprint5-85 branch September 17, 2020 13:58
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

3 participants