Improvements to Site Archive tool

Activity

SAKTRUNK-44 9

Keyboard shortcuts  
Summarize the review outcomes (optional)
 
#permalink

Details

Warning: no files are visible, they have all been filtered.
Participant Role Time Spent Comments Latest Comment
Author 38m 4 Matthew, are you saying that merge() needs the following?...
Reviewer - 73% reviewed 9m    
Reviewer - 50% reviewed 2m    
Reviewer - 12% reviewed 7m    
Reviewer - 35% reviewed 26m 5 I think so, sorry, I don't really have time to look into ...
Reviewer - 0% reviewed      
Total   1h 21m 9  
#permalink

Objectives

https://jira.sakaiproject.org/browse/SAK-25867

Improvements to Site Archive tool

Branches in review

#permalink

Issues Raised From Comments

Key Summary State Assignee
#permalink

General Comments

Matthew Buckett

Am I right in thinking that in the end the SiteMerger calls org.sakaiproject....

Am I right in thinking that in the end the SiteMerger calls org.sakaiproject.site.impl.BaseSiteService#merge(java.lang.String, org.w3c.dom.Element, java.lang.String) to import the site?

If so the permission checks done in that method currently aren't in line with the permission checks done in the standard add methods:

org.sakaiproject.site.impl.BaseSiteService#addSite(java.lang.String, org.sakaiproject.site.api.Site)

Nicola Monat-Jacobs

Matthew, are you saying that merge() needs the following? // check security ...

Matthew, are you saying that merge() needs the following?

// check security (throws if not permitted)
if (isUserSite(id))
{
	unlock(SECURE_ADD_USER_SITE, siteReference(id));
}
else
{
	unlock(SECURE_ADD_SITE, siteReference(id));
}

// SAK=12631
if ( isCourseSite(other.getId()) ) {
	unlock(SECURE_ADD_COURSE_SITE, siteReference(id));			
}

// KNL-703
if ( isPortfolioSite(other.getId()) ) {
	unlock(SECURE_ADD_PORTFOLIO_SITE, siteReference(id));			
}

Matthew Buckett

I think so, sorry, I don't really have time to look into this in detail, but ...

I think so, sorry, I don't really have time to look into this in detail, but I would expect the same restrictions to be placed on creating a new site if you are manually creating a new one, or importing a site from another system. Is there a reason why the merge shouldn't do the more complex new site checks?

/site-manage-tool/.../bundle/sitesetupgeneric.properties Changed
Open in IDE #permalink
/site-manage-tool/.../tool/SiteAction.java Changed 4
Open in IDE #permalink
/site-manage-tool/tool/.../css/site-manage.css Changed
Open in IDE #permalink
/site-manage-tool/tool/.../js/site-manage.js Changed
Open in IDE #permalink
/site-manage-tool/.../sitesetup/chef_site-editToolGroupFeatures.vm Changed
Open in IDE #permalink
/site-manage-tool/.../sitesetup/chef_site-newSiteConfirm.vm Changed
Open in IDE #permalink
/site-manage-tool/.../sitesetup/chef_site-type.vm Changed
Open in IDE #permalink
/site-manage-tool/.../sitesetup/chef_site-uploadArchive.vm Added
Open in IDE #permalink
/site-manage-tool/tool/pom.xml Changed
Open in IDE #permalink
/src/main/.../mock/service/SiteService.java Changed  
Open in IDE #permalink
/api/src/main/.../site/api/SiteService.java Changed  
Open in IDE #permalink
/api/src/main/.../site/cover/SiteService.java Changed  
Open in IDE #permalink
/kernel-impl/src/.../impl/BaseSiteService.java Changed  
Open in IDE #permalink
/kernel-impl/src/.../hsqldb/sakai_realm.sql Changed  
Open in IDE #permalink
/kernel-impl/src/.../sql/mysql/sakai_realm.sql Changed  
Open in IDE #permalink
/kernel-impl/src/.../oracle/sakai_realm.sql Changed  
Open in IDE #permalink
/archive-api/src/.../api/ArchiveService.java Changed  
Open in IDE #permalink
/archive-api/src/.../archive/api/ZipAware.java Added  
Open in IDE #permalink
/archive-impl/.../impl/ArchiveService2Impl.java Changed  
Open in IDE #permalink
/archive-impl/impl2/.../impl/SiteZipper.java Added  
Open in IDE #permalink
/archive-impl/impl2/pom.xml Changed  
Open in IDE #permalink
/archive-impl/pack/.../WEB-INF/components.xml Changed  
Open in IDE #permalink
/pom.xml Changed   2
Open in IDE #permalink
/archive-tool/tool/.../bundle/admin.properties Changed  
Open in IDE #permalink
/archive-tool/tool/.../tool/ArchiveAction.java Changed  
Open in IDE #permalink
/archive-tool/.../tool/DownloadServlet.java Added  
Open in IDE #permalink
/archive-tool/tool/.../webapp/WEB-INF/web.xml Changed  
Open in IDE #permalink
/archive-tool/.../archive/chef_archive-batch-archive-confirm.vm Added  
Open in IDE #permalink
/archive-tool/.../archive/chef_archive-batch.vm Changed  
Open in IDE #permalink
/archive-tool/.../archive/chef_archive-download.vm Added  
Open in IDE #permalink
/archive-tool/tool/.../archive/chef_archive.vm Changed  
Open in IDE #permalink
/archive-tool/.../archive/chef_archive_noaccess.vm Changed  
Open in IDE #permalink
/archive-tool/.../webapp/velocity.properties Changed  
Open in IDE #permalink
/archive-tool/tool/pom.xml Changed  
Open in IDE #permalink
/pom.xml Changed  
Open in IDE #permalink

Review updated: Reload | Ignore | Collapse

You cannot reload the review while writing a comment.

Create issue

X
Assign To Me

Log time against