SAK-52322 Samigo fix FK on SAM_EXTENDEDTIME_T blocks delete of publis…

[ottenhoff]

…hed assessment during site import cleanup

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Extended time entries are now properly removed when their associated assessments are deleted.

[coderabbitai]

Walkthrough

Introduces batch deletion functionality for ExtendedTime entries across the assessment facade layer. A new public method deleteEntriesForPub is added to enable deletion of all ExtendedTime entries associated with a specific PublishedAssessment, with integration into the publication deletion workflow.

Changes

Cohort / File(s)	Summary
ExtendedTime Deletion API samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/ExtendedTimeQueriesAPI.java, samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/ExtendedTimeFacade.java, samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/ExtendedTimeQueries.java	Adds new public method deleteEntriesForPub(PublishedAssessmentIfc pub) across interface, facade, and queries layers. Implementation includes null checks, logging, bulk delete operation by publishedAssessmentId, and exception handling returning boolean status.
Assessment Deletion Integration samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/PublishedAssessmentFacadeQueries.java	Integrates the new ExtendedTime deletion method into the publication deletion path, calling ExtendedTimeFacade.deleteEntriesForPub(data) prior to deleting the PublishedAssessmentData.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the specific issue (SAK-52322), the component (Samigo), and the main problem (FK on SAM_EXTENDEDTIME_T blocks delete of published assessment), accurately summarizing the changeset's primary purpose.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Important

Action Needed: IP Allowlist Update

If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:

• ✨ 136.113.208.247/32 (new)
• 34.170.211.100/32
• 35.222.179.152/32

Failure to add the new IP will result in interrupted reviews.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/PublishedAssessmentFacadeQueries.java`:
- Around line 1867-1871: The call to
ExtendedTimeFacade.deleteEntriesForPub(data) currently ignores its boolean
return value; change the logic in the deletion block so that you check the
boolean result from ExtendedTimeFacade.deleteEntriesForPub(...) and abort the
published-assessment deletion if it returns false (do not call
getHibernateTemplate().delete(data) on failure). Use the existing
ExtendedTimeFacade reference (ExtendedTimeFacade extendedTimeFacade =
PersistenceService.getInstance().getExtendedTimeFacade()), the
deleteEntriesForPub(...) result, and the getHibernateTemplate().delete(data)
call to gate the deletion (e.g., return/throw/propagate an error when cleanup
fails) so FK/orphan issues are avoided.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Abort deletion when ExtendedTime cleanup fails.

deleteEntriesForPub returns a boolean but the result is ignored. If cleanup fails, you can still attempt to delete the published assessment, which risks FK violations or orphaned ExtendedTime rows. Consider failing fast when cleanup fails.

✅ Suggested fix

 				ExtendedTimeFacade extendedTimeFacade = PersistenceService.getInstance().getExtendedTimeFacade();
 				if (extendedTimeFacade != null) {
-					extendedTimeFacade.deleteEntriesForPub(data);
+					boolean extendedTimeDeleted = extendedTimeFacade.deleteEntriesForPub(data);
+					if (!extendedTimeDeleted) {
+						log.warn("Unable to delete ExtendedTime entries for published assessment id: {}", data.getPublishedAssessmentId());
+						throw new IllegalStateException("Failed to delete ExtendedTime entries for published assessment id " + data.getPublishedAssessmentId());
+					}
 				}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	ExtendedTimeFacade extendedTimeFacade = PersistenceService.getInstance().getExtendedTimeFacade();
	if (extendedTimeFacade != null) {
	extendedTimeFacade.deleteEntriesForPub(data);
	}
	getHibernateTemplate().delete(data);
	ExtendedTimeFacade extendedTimeFacade = PersistenceService.getInstance().getExtendedTimeFacade();
	if (extendedTimeFacade != null) {
	boolean extendedTimeDeleted = extendedTimeFacade.deleteEntriesForPub(data);
	if (!extendedTimeDeleted) {
	log.warn("Unable to delete ExtendedTime entries for published assessment id: {}", data.getPublishedAssessmentId());
	throw new IllegalStateException("Failed to delete ExtendedTime entries for published assessment id " + data.getPublishedAssessmentId());
	}
	}
	getHibernateTemplate().delete(data);

🤖 Prompt for AI Agents

In
`@samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/facade/PublishedAssessmentFacadeQueries.java`
around lines 1867 - 1871, The call to
ExtendedTimeFacade.deleteEntriesForPub(data) currently ignores its boolean
return value; change the logic in the deletion block so that you check the
boolean result from ExtendedTimeFacade.deleteEntriesForPub(...) and abort the
published-assessment deletion if it returns false (do not call
getHibernateTemplate().delete(data) on failure). Use the existing
ExtendedTimeFacade reference (ExtendedTimeFacade extendedTimeFacade =
PersistenceService.getInstance().getExtendedTimeFacade()), the
deleteEntriesForPub(...) result, and the getHibernateTemplate().delete(data)
call to gate the deletion (e.g., return/throw/propagate an error when cleanup
fails) so FK/orphan issues are avoided.