-
Notifications
You must be signed in to change notification settings - Fork 56
Data model for spend price per building blocks and project settings #373
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
Conversation
# Conflicts: # package-lock.json # postman.json # src/constants.js # src/permissions/index.js # src/routes/index.js # swagger.yaml Also fixed lint errors.
…ature/price-estimation-items/merged # Conflicts: # postman.json # src/constants.js # src/models/projectEstimationItem.js # src/permissions/index.js # src/routes/index.js # src/routes/projects/create.js # swagger.yaml
…d out code and added warning comments.
…erformance when retrieving ProjectSettings and checking their permissions added warning comments
Thanks @maxceem for the PR, a quick question for you: Do you think if we merge this PR into dev, we can safely move the changes to production, in case we have to do a production release before the |
I think yes. This PR almost doesn't touch the existent logic. The only place is triggering ProjectEstimationItems calculation on project creation. But I think we have enough time to verify there are no issues in project creation before releasing. All other logic uses new models and endpoints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Few queries and minor suggestions. @maxceem could you please refer to me our final approach about using settings vs building blocks for determining pricing items? I would like keep that handy for my discussions with the team.
Thanks for the changes @maxceem Seems like we have conflicts now. |
# Conflicts: # postman.json # src/permissions/index.js
@vikasrohit resolved merge conflicts. |
I've created a wiki page with summarised implementation logic https://github.com/topcoder-platform/tc-project-service/wiki/Logic-for-calculating-ProjectEstimationItems Let me know if any moment has to be clarified. |
Implementation for #316
Done via 2 challenges:
http://www.topcoder.com/challenge-details/30096338/?type=develop
http://www.topcoder.com/challenge-details/30096337/?type=develop
Implemented:
Models:
ProjectEstimationItems
ProjectSettings
BuildingBlock
Endpoints:
GET /projects/{id}/estimations/{id}/items
ProjectSettings
BuildingBlock
are returned now as a par ofGET /projects/metadata
ProjectEstimationItems calculation
ProjectEstimationItems are calculated when:
estimations
and there arebuildingBlocks
for that estimations. Note, we don't have any logic to recalculateProjectEstimationItems
basedbuildingBlocks
. SoProjectEstimationItems
are calculated based onbuildingBlocks
only during project creation.ProjectEstimationItems
are deleted and recreated usingProjectSettings
. And later when add/update/delete ProjectSettings about pricing all existentProjectEstimationItems
are delted and recreated.As a result of the logic above, if we have at least one ProjectSettings about pricing
buildingBlocks
are not more used to calculateProjectEstimationItems
.SQL Migrations
To apply this PR two migrations SQL scripts should be applied: