You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by niteshsarda <gi...@git.apache.org> on 2017/03/03 11:29:14 UTC

[GitHub] cloudstack pull request #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain...

GitHub user niteshsarda opened a pull request:

    https://github.com/apache/cloudstack/pull/1987

    CLOUDSTACK-9814 : Unable to edit a Sub domain, which has the same name in different domains

    ISSUE
    ============
    Unable to edit a Sub domain, which has the same name in different domains.
    
    TROUBLESHOOTING
    ==================
    1. Created two sub-domains with same name. Under two different domains.
    ROOT
    Domain 1 : Sample
    AA000001
    BB000001
    
    Domain 2 : Test
    AA000001
    
    2. Try to edit resource - Click on edit - edit any parameter.
    
    EXPECTED BEHAVIOR
    ==================
    It should allow to edit the required parameter and update the value.
    
    ACTUAL BEHAVIOR
    ==================
    It throws an error "Failed to update specified domain id with name 'AA000001' since it already exists in system. 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Accelerite/cloudstack SubDomainIssue

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1987.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1987
    
----
commit 2d29b4fc48081efe683457c7f4597c643f3fa480
Author: Nitesh Sarda <ni...@accelerite.com>
Date:   2017-03-03T11:24:48Z

    CLOUDSTACK-9814 : Unable to edit a Sub domain, which has the same name in different domains

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain...

Posted by niteshsarda <gi...@git.apache.org>.
Github user niteshsarda commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1987#discussion_r104283067
  
    --- Diff: server/src/org/apache/cloudstack/region/RegionManagerImpl.java ---
    @@ -229,7 +232,7 @@ public boolean deleteUser(DeleteUserCmd cmd) {
          */
         @Override
         public Domain updateDomain(UpdateDomainCmd cmd) {
    -        return _domainMgr.updateDomain(cmd);
    +        return _managementService.updateDomain(cmd);
    --- End diff --
    
    @yvsubhash : As per your suggestion, I have changed the search query and also removed the unused method updateDomain from managementserviceimpl class.
    
    Please check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain, which...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the issue:

    https://github.com/apache/cloudstack/pull/1987
  
    @niteshsarda thank you very much :)
    The less duplicated or useless code around the better.
    
    The method 'updateDomain' is huge and you just changed a single line there, so I will not discuss/suggest any improvements there.
    
    I am ok with this PR LGTM.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain, which...

Posted by niteshsarda <gi...@git.apache.org>.
Github user niteshsarda commented on the issue:

    https://github.com/apache/cloudstack/pull/1987
  
    @rafaelweingartner : As per your suggestion, I have changed the search query and also removed the unused method updateDomain from managementserviceimpl class.
    
    Please check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain, which...

Posted by niteshsarda <gi...@git.apache.org>.
Github user niteshsarda commented on the issue:

    https://github.com/apache/cloudstack/pull/1987
  
    This is Ready to Merge


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain, which...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the issue:

    https://github.com/apache/cloudstack/pull/1987
  
    @niteshsarda I suggest you adding line 2281 from "managementService" to "domainMgr". The only difference between those two methods is this line. Assuming that you changed the method calls, and then everything started working. 
    
    Then, please delete the com.cloud.server.ManagementServerImpl.updateDomain(UpdateDomainCmd), it is not used. It is incredible to have a method with ~100 duplicated between classes.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain...

Posted by yvsubhash <gi...@git.apache.org>.
Github user yvsubhash commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1987#discussion_r104140435
  
    --- Diff: server/src/org/apache/cloudstack/region/RegionManagerImpl.java ---
    @@ -229,7 +232,7 @@ public boolean deleteUser(DeleteUserCmd cmd) {
          */
         @Override
         public Domain updateDomain(UpdateDomainCmd cmd) {
    -        return _domainMgr.updateDomain(cmd);
    +        return _managementService.updateDomain(cmd);
    --- End diff --
    
    Instead of changing the method that is getting called, the missing one in search query can be added here and it seems the updateDomain method in managementserviceimpl is not in use. It can be removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1987: CLOUDSTACK-9814 : Unable to edit a Sub domain, which...

Posted by jayantpatil1234 <gi...@git.apache.org>.
Github user jayantpatil1234 commented on the issue:

    https://github.com/apache/cloudstack/pull/1987
  
    I have tested this and **LGTM** for test.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---