Skip to content

Conversation

@maxceem
Copy link
Contributor

@maxceem maxceem commented Apr 12, 2019

winning submission from challenge 30088320 - Topcoder Project Service - Product templates use forms

Submission looks good to me.

I think to finish with this refactoring process we can do a couple of more things:

  1. In this PR helper method has been created util.checkModel while in previous code this method has been defined several times:

    So we can reuse a new helper method in all these cases.

  2. At the moment we have two ways of defining forms for both ProductTemplates and ProjectTemplates: old way inside the template and new way using Form model.
    So both of fields are optional:

    image

    and

    image

    For ProductTemplate we should check in create/update endpoints that

    • either form or template field is defined, but not both together.

    For ProjectTemplate we should check in create/update endpoints that:

    • either scope or form field must be defined, but not both together
    • scope or priceConfig cannot be defined together (they can be both null though)
    • either phases or phaseConfig field must be defined, but not both together.

@maxceem maxceem requested a review from vikasrohit April 12, 2019 08:45
@vikasrohit
Copy link

-form or priceConfig cannot be defined together (they can be both null though)

@maxceem I didn't get this statement. IMO, form and priceConfig would be set together in fact.

@vikasrohit vikasrohit merged commit 29ed9d4 into dev Apr 25, 2019
@maxceem
Copy link
Contributor Author

maxceem commented Apr 25, 2019

@vikasrohit right, it was a typo, it should be:

  • scope or priceConfig cannot be defined together (they can be both null though)

I've updated in the comment above.

The main idea that we should validate that we define form template either new or old way, but not both together.

@vikasrohit
Copy link

Okay, got it. Please feel free to run task for the pending items as you listed above, they look good to me.

@vikasrohit vikasrohit deleted the feature/product-templates-refactoring branch July 29, 2019 06:26
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.

3 participants