Gilad Chaplik has posted comments on this change.

Change subject: core: extend DiskProfile.getAllForStorageDomain to a user query
......................................................................


Patch Set 6:

(3 comments)

new patch to follow

http://gerrit.ovirt.org/#/c/29812/6/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/profiles/DiskProfileDao.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/profiles/DiskProfileDao.java:

Line 33:      *            the storage domain's ID
Line 34:      * @param userId
Line 35:      *            the user's ID
Line 36:      * @param isFiltered
Line 37:      *            Whether the results should be filtered according to 
the user's permissions
> /s/Whether/indicating whether
Done
Line 38:      * @return the list of disk profiles
Line 39:      */
Line 40:     List<DiskProfile> getAllForStorageDomain(Guid storageDomainId, 
Guid userId, boolean isFiltered);
Line 41: 


http://gerrit.ovirt.org/#/c/29812/6/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/profiles/DiskProfileDaoTest.java
File 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/profiles/DiskProfileDaoTest.java:

Line 152:     public void testGetFilteredByPermissions() {
Line 153:         List<DiskProfile> result =
Line 154:                 
dao.getAllForStorageDomain(FixturesTool.STORAGE_DOAMIN_SCALE_SD5, 
PRIVILEGED_USER_ID, true);
Line 155:         assertNotNull(result);
Line 156:         assertEquals(2, result.size());
> I'd export the verification from testGetAllForStorageDomainFull() to a meth
Done
Line 157:     }
Line 158: 
Line 159:     @Test
Line 160:     public void testGetFilteredByPermissionsForUnprivilegedUser() {


Line 159:     @Test
Line 160:     public void testGetFilteredByPermissionsForUnprivilegedUser() {
Line 161:         List<DiskProfile> result =
Line 162:                 
dao.getAllForStorageDomain(FixturesTool.STORAGE_DOAMIN_SCALE_SD5, 
UNPRIVILEGED_USER_ID, true);
Line 163:         assertEquals(0, result.size());
> I'd replace with assertTrue(result.isEmpty())
Done
Line 164:     }


-- 
To view, visit http://gerrit.ovirt.org/29812
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bb75728cd76bba7e39de2314275f5bc87942c6b
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to