Challenge Overview
As submission of the challenge https://www.topcoder.com/challenges/30086308/?type=develop didn’t pass the minimal score but still has a good quality base to continue we would like to accomplish the missing requirements as a private task.
This private task covers the winning prize of the initial challenge and increased as the scope of the challenge appeared to be bigger than initially evaluated.
Individual requirements
The main aim of this private task is to complete the requirements which were initially described in the challenge https://www.topcoder.com/challenges/30086308/?type=develop.
-
You may use this fixed version of seedMetadata https://github.com/topcoder-platform/tc-project-service/blob/feature/handle-errors-seed-metadata/migrations/seedMetadata.js to populate metadata in the DB. How to run it, see https://github.com/topcoder-platform/tc-project-service#import-sample-metadata--projects. Note that this script works before refactoring done in this challenge.
-
DB Schema: form, priceConfig and planConfig tables should have key as UNIQUE - it’s done.
-
But version and revision should not be UNIQUE.
-
A combination of key, version and revision should be UNIQUE.
-
-
When we create a new projectTemplate we should check if referred form, priceConfig and planConfig exists, otherwise return an error.
-
if form, priceConfig and planConfig is just not defined - it’s ok, just create projectTemplate
-
if form, priceConfig or planConfig is defined as key only {“key”: “dev”}, then we check that form, priceConfig or planConfig exists with such a key, otherwise return error code 422 “Form with key “dev” referred in the project template is not found” (instead of “Form” in the message could be “Price Config” or “Plan config”)
-
if form, priceConfig or planConfig is defined by key and version {“key”: “dev”, “version”: 123}, then we check that form, priceConfig or planConfig exists with such a key and version combination, otherwise return error with code 422 “Form with key “dev” and version “123” referred in the project template is not found” (instead of “Form” in the message could be “Price Config” or “Plan config”)
-
-
All three models implement the same methods deleteOldestRevision, latestVersion, newVersionNumber, createNewVersion, latestVersionIncludeUsed. Can we implement them once and reuse in all of these models?
-
Fix swagger errors:
-
https://drive.google.com/file/d/14YlzIkPw7cZ9CMVji67PgJiyPKh_4J-n/view
-
GET endpoints provide documentation for 422-Invalid Input status code, there is no input provided for GET endpoints.
-
All POST endpoints - when the request is sucessful it returns 201-Created, but it is documented in swagger to return 200-OK.
-
-
Postman test "Form Version -> get latest version" generates ISE-500 when there is no data in the database. See : https://drive.google.com/open?id=1QsewXYWWakPi01ljpN3FZUwdHuxWumCq It should return 404-Not Found instead
-
"Price Config Version -> get latest version" - has the same issue as above.
-
"Plan Config Version -> get latest version" - has the same issue as above.
-
-
Postman test “Form Version -> list form” returns success even when forms with such key doesn’t exits. If we call “/v4/projects/metadata/form/dev/versions” and no forms with key “dev” exist, we should return 404.
-
same for priceConfig and planConfig.
-
-
All error messages about form, priceConfig and planConfig which happens to particular key and version, should mention key and version. Now in some cases, such error mention only key. For example, delete form has message `Form not found for key ${req.params.key}`. But it should be `Form not found for key ${req.params.key} version ${req.params.version}`. Same in files:
-
delete form version
-
get form version
-
delete planConfig version
-
get planConfig version
-
delete priceConfig version
-
get priceConfig version
-
-
Some error message has no space between work key and key value, like "Form not found for keydev version 1 reversion 4"
-
When we upgrade template which has null scope or phase the error message now is "Current project template's scope or phases is null undefined". Should be fixed to no say “undefined”.
-
GET /v4/projects/metadata/form/invalidKey/versions/1/revisions - returns 200-OK, it should return 404-Not Found instead ( invalidKey does not exist). As per the swagger documentation 404-Not found should be returned in this case.
-
PATCH /v4/projects/metadata/form/{key}/versions/1 generates an error with the error message "Configuration property \"MAX_REVISON_NUMBER\" is not defined". See https://drive.google.com/open?id=1q2WQCdyeuN3aFN3LQmETMhpqR16Runmt
-
REVISON -> REVISION ( in many places in the code)
-
GET /v4/projects/metadata/form/{key} - The implementation of this endpoint is incorrect. It returns the latest revision among all versions, however the requirement is to return the latest version ( latest revision of the latest version).
-
To check the issue follow these steps :
-
1. create version 1 of the form
-
2. create version 2 of the form
-
3. update version 1 of the form ( another revision)
-
The GET /v4/projects/metadata/form/{key} In the scenario above should return the latest version ( version 2), The current implementation returns the latest revision of version 1 which is incorrect.
-
-
GET /v4/projects/metadata/priceConfig/{key} has the same issue as above.
-
GET /v4/projects/metadata/planConfig/{key} has the same issue as above.
-
-
form/version/update.js line #45 - should use '>=' instead of '>'. I tested with MAX_REVISION_NUMBER=5 and got 6 (not deleted) Revisions in the DB, see https://drive.google.com/open?id=12L6XdRL92xmZbg6hzgoR6QykukVzGDFz
-
planConfig and priceConfig have the same issue as above.
-
-
Not sure how to test/validate the /v4/projects/metadata?includeAllRefered=true endpoint I cannot create project templates referencing existing forms, price config and plan config using the provided postman tests.
-
Create additional postman endpoints so we can create parojectTemplates which refer to forms, priceConfigs and planConfigs. So we can test the endpoint with includeAllRefered in postman.
-
-
projectTemplates/upgrade.js - permissions 'productTemplate.upgrade' used is incorrect, it should be 'projectTemplate.upgrade' instead.
-
"3. Create a migration script" - The implementation of the upgrade script is incorrect :
-
-- For form when not specified it should copy fields sections, wizard and preparedConditions from projectTemplate.scope, the provided implementation copies all the project template scope to form scope.
-
-- For PriceConfig when not specified, it should copy all fields except of sections and wizard from pt.scope to priceConfig.scope, the provided implementation simply copies all the scope.
-
-
When we upgrade and we define existent forms, priceConfigs and planConfigs we should check that they exist and return error if some defined forms, priceConfigs or planConfigs doesn’t exist.
-
Create a test case in postman when some of forms, priceConfigs or planConfigs doesn’t exist.
-
-
models/form.js - The exported funtion name 'definePhaseProduct' is confusing.
-
planConfig.js and priceConfig.js have the same issue as above, arrow functions could have been used, the function name is not needed.
-
-
src/routes/form/revision/delete.js - 'reversion' -> 'revision'
-
The typo mentioned in response above should be fixed in all the following files as well :
-
-- src/routes/form/revision/delete.spec.js
-
-- src/routes/form/revision/get.js
-
-- src/routes/planConfig/revision/delete.js
-
-- src/routes/planConfig/revision/delete.spec.js
-
-- src/routes/planConfig/revision/get.js
-
-- src/routes/priceConfig/revision/delete.js
-
-- src/routes/priceConfig/revision/delete.spec.js
-
-- src/routes/priceConfig/revision/get.js
-
-
-
Postman issues
-
Postman - 'Form Revision -> get a particular revision' : The endpoint is incorrect, it is for getting a particular version. (i.e '/revisions/{revisionId}' should be added to the endpoint)
-
Postman - 'Price Config Revision -> get a particular revision' has the same issue as above.
-
Postman - 'Plan Config Revision -> get a particular revision' has the same issue as above.
-
-
Postman improvements
-
Some tests in Posman use “key”=“dev” and some use “boom1”. Let’s use one key “dev” everywhere where it makes sense so we can easily run all the test in postman without changing endpoint. For example, now we can run “Create form” version, but if after we run “list froms” we got nothing, as we use different keys there. Same for other endpoints.
-
Create a test for projectTemplates which will create a new projectTemplate which would refer to form, priceConfig and planConfig.
-
Create a test for projectTemplate which would upgrade template without defining form, priceConfig and planConfig.
-
-
Postman language fixes:
-
Name everything starting from a capital letter for consistency, as it’s done for all other sections: https://monosnap.com/file/eCc7V4NWpAuSEwHRJVurP3I0A2svnk
-
'Price Config Revision ' : 'create form' should be 'create price config'
-
'Plan Config Revision' : 'create form' should be 'create plan config'
-
‘Form Version’:’list form’ should be ‘List forms’
-
-
Move “upgrade project template” route close to other project templates routes in src/routes/index.js
-
Don’t disable eslint rule /* eslint-disable no-param-reassign */ adjust code instead to follow this rule.
-
I made a typo in the name of “includeAllRefered”, please, rename it to “includeAllReferred” (double “r”) everywhere.
-
Add next unit tests:
-
Check that when we update a version of form, priceConfig or planConfig - we get a new revision
-
Check that when we get the latest version of form, priceConfig or planConfig - we get the latest version (as now we get the latest revision instead)
-
General requirements
-
Implement unit test for all these new endpoints similar way as for existent endpoints
-
Update postman collection to reflect all the changes
-
Update swagger to reflect all the changes
-
Lint should pass
-
Existent unit tests should pass
Final Submission Guidelines
-
Patch to the dev branch of Project Service (Please, avoid errors in the patch)
-
Text file with the hash of the commit you use for the patch.