Skip to content
This repository was archived by the owner on Mar 8, 2020. It is now read-only.

Conversation

@shachisharma
Copy link

Checklist

  • [Y ] A link to the issue/user story that the pull request relates to
  • [ Y] How to recreate the problem without the fix
  • [ Y] Design of the fix
  • [ Y] How to prove that the fix works
  • [ Y] Automated tests that prove the fix keeps on working
  • Documentation - any JSDoc, website, or Stackoverflow answers?

Issue/User story

When a model is created then namespaces are defined by user in model file. There is no check in composer to test duplicate namespace. Issue #1174

Steps to Reproduce

  1. Create a model file with addition of a namespace twice, run command to create archive. The archive will be created without any error or messages.

Existing issues

https://github.com/hyperledger/composer/issues/1189

Design of the fix

While adding the namespace when model is created, a check has been added whether namespace already exists or not. If exists, an error is thrown otherwise namespace is added. This is a logical point to add this kind of check.

Validation of the fix

a code review has been done

Automated Tests

The four tests has been added to confirm that the fix is working. The change is in two functions. Two testcases for each function has been added.

  1. add a namespace for string, then again try to add second one with same namespace. Composer will throw an error "namespace already exists"
    2.add a list of namespaces (with some namespace repeated) for a string, the composer will automatically check for duplicate namespace and throw an error "namespace already exists"

repeat above two cases when namespace is specified as object

What documentation has been provided for this pull request

@CLAassistant
Copy link

CLAassistant commented Jul 15, 2017

CLA assistant check
All committers have signed the CLA.

@dselman
Copy link
Contributor

dselman commented Jul 20, 2017

Please sign the CLA.

@shachisharma
Copy link
Author

I have signed the CLA the day I have created the PR. If I click onto "Details" it shows me CLA signed. But still in the PR it is showing CLA as unsigned, not sure what's the problem

@sstone1
Copy link
Contributor

sstone1 commented Jul 20, 2017

@shachisharma you are logged in using the shachisharma ID but your git commit is from the shacshar ID. CLA tracking is done based on git commits, so you must sign the CLA as shacshar.

@nklincoln nklincoln requested a review from dselman July 20, 2017 12:40
modelFiles.push(modelFile);
}
modelManager.addModelFiles(modelFiles, modelFileNames);
//modelManager.addModelFiles(modelFiles, modelFileNames);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove dead code

Copy link
Contributor

@dselman dselman left a comment

Choose a reason for hiding this comment

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

Please remove the commented code. The rest looks good. Thanks!

@shachisharma
Copy link
Author

I have created new PR #1629 corresponding to this one as I was not able to sign CLA without it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants