-
Notifications
You must be signed in to change notification settings - Fork 269
BAH-4874 Draft forms #339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
BAH-4874 Draft forms #339
Changes from all commits
024d384
b3d9e0b
26fe105
8567fac
127f382
2c4b2fe
146ccb5
9e66f74
c612b71
7fa97c7
0943abf
5103bcc
5bc8f81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package org.bahmni.module.bahmnicore.contract; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| public class FormDraftRequest { | ||
|
|
||
| @JsonProperty | ||
| private String patientUuid; | ||
|
|
||
| @JsonProperty | ||
| private String providerUuid; | ||
|
|
||
| @JsonProperty | ||
| private String formData; | ||
|
|
||
| public FormDraftRequest() { | ||
| } | ||
|
|
||
| public String getPatientUuid() { | ||
| return patientUuid; | ||
| } | ||
|
|
||
| public void setPatientUuid(String patientUuid) { | ||
| this.patientUuid = patientUuid; | ||
| } | ||
|
|
||
| public String getProviderUuid() { | ||
| return providerUuid; | ||
| } | ||
|
|
||
| public void setProviderUuid(String providerUuid) { | ||
| this.providerUuid = providerUuid; | ||
| } | ||
|
|
||
| public String getFormData() { | ||
| return formData; | ||
| } | ||
|
|
||
| public void setFormData(String formData) { | ||
| this.formData = formData; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package org.bahmni.module.bahmnicore.contract; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import java.util.Date; | ||
|
|
||
| public class FormDraftResponse { | ||
|
|
||
| @JsonProperty | ||
| private String uuid; | ||
|
|
||
| @JsonProperty | ||
| private String formData; | ||
|
|
||
| @JsonProperty | ||
| private Boolean markedAsSaved; | ||
|
|
||
| @JsonProperty | ||
| private Long timestamp; | ||
|
|
||
| public FormDraftResponse() { | ||
| } | ||
|
|
||
| public FormDraftResponse(String uuid) { | ||
| this.uuid = uuid; | ||
| } | ||
|
|
||
| public FormDraftResponse(String uuid, String formData) { | ||
| this.uuid = uuid; | ||
| this.formData = formData; | ||
| } | ||
|
|
||
| public FormDraftResponse(String uuid, String formData, Boolean markedAsSaved) { | ||
| this.uuid = uuid; | ||
| this.formData = formData; | ||
| this.markedAsSaved = markedAsSaved; | ||
| } | ||
|
|
||
| public FormDraftResponse(String uuid, String formData, Boolean markedAsSaved, Long timestamp) { | ||
| this.uuid = uuid; | ||
| this.formData = formData; | ||
| this.markedAsSaved = markedAsSaved; | ||
| this.timestamp = timestamp; | ||
| } | ||
|
|
||
| public String getUuid() { | ||
| return uuid; | ||
| } | ||
|
|
||
| public void setUuid(String uuid) { | ||
| this.uuid = uuid; | ||
| } | ||
|
|
||
| public String getFormData() { | ||
| return formData; | ||
| } | ||
|
|
||
| public void setFormData(String formData) { | ||
| this.formData = formData; | ||
| } | ||
|
|
||
| public Boolean getMarkedAsSaved() { | ||
| return markedAsSaved; | ||
| } | ||
|
|
||
| public void setMarkedAsSaved(Boolean markedAsSaved) { | ||
| this.markedAsSaved = markedAsSaved; | ||
| } | ||
|
|
||
| public Long getTimestamp() { | ||
| return timestamp; | ||
| } | ||
|
|
||
| public void setTimestamp(Long timestamp) { | ||
| this.timestamp = timestamp; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package org.bahmni.module.bahmnicore.contract; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonInclude; | ||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| @JsonInclude(JsonInclude.Include.NON_NULL) | ||
| public class FormDraftSummaryResponse { | ||
|
|
||
| @JsonProperty | ||
| private String draftUuid; | ||
|
|
||
| @JsonProperty | ||
| private String patientUuid; | ||
|
|
||
| @JsonProperty | ||
| private String patientName; | ||
|
|
||
| @JsonProperty | ||
| private String patientIdentifier; | ||
|
|
||
| @JsonProperty | ||
| private String formName; | ||
|
|
||
| @JsonProperty | ||
| private Long timestamp; | ||
|
|
||
| public FormDraftSummaryResponse() { | ||
| } | ||
|
|
||
| public String getDraftUuid() { | ||
| return draftUuid; | ||
| } | ||
|
|
||
| public void setDraftUuid(String draftUuid) { | ||
| this.draftUuid = draftUuid; | ||
| } | ||
|
|
||
| public String getPatientUuid() { | ||
| return patientUuid; | ||
| } | ||
|
|
||
| public void setPatientUuid(String patientUuid) { | ||
| this.patientUuid = patientUuid; | ||
| } | ||
|
|
||
| public String getPatientName() { | ||
| return patientName; | ||
| } | ||
|
|
||
| public void setPatientName(String patientName) { | ||
| this.patientName = patientName; | ||
| } | ||
|
|
||
| public String getPatientIdentifier() { | ||
| return patientIdentifier; | ||
| } | ||
|
|
||
| public void setPatientIdentifier(String patientIdentifier) { | ||
| this.patientIdentifier = patientIdentifier; | ||
| } | ||
|
|
||
| public String getFormName() { | ||
| return formName; | ||
| } | ||
|
|
||
| public void setFormName(String formName) { | ||
| this.formName = formName; | ||
| } | ||
|
|
||
| public Long getTimestamp() { | ||
| return timestamp; | ||
| } | ||
|
|
||
| public void setTimestamp(Long timestamp) { | ||
| this.timestamp = timestamp; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package org.bahmni.module.bahmnicore.dao; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.bahmni.module.bahmnicore.model.FormDraft; | ||
|
|
||
| public interface FormDraftDAO { | ||
|
|
||
| /** | ||
| * Save a new form draft. | ||
| * Each draft gets a unique UUID and is never updated once voided (new drafts are created instead). | ||
| * | ||
| * @param draft the FormDraft object to save or update | ||
| * @return the saved FormDraft object | ||
| */ | ||
| FormDraft saveOrUpdate(FormDraft draft); | ||
|
|
||
| /** | ||
| * Retrieve the latest non-voided form draft for a patient and user. | ||
| * | ||
| * @param patientId the OpenMRS patient ID | ||
| * @param userId the OpenMRS user ID (provider) | ||
| * @return the latest non-voided FormDraft object, or null if not found | ||
| */ | ||
| FormDraft getLatestByPatientAndUser(Integer patientId, Integer userId); | ||
|
|
||
| /** | ||
| * Soft delete (void) the latest non-voided form draft for a patient and user. | ||
| * Sets voided = true and dateVoided = now, voidedBy = currentUser, voidReason = "Draft deleted" | ||
| * | ||
| * @param patientId the OpenMRS patient ID | ||
| * @param userId the OpenMRS user ID (provider) | ||
| */ | ||
| void deleteLatestDraft(Integer patientId, Integer userId); | ||
|
|
||
| /** | ||
| * Soft delete (void) all non-voided form drafts. | ||
| * Sets voided = true and dateVoided = now, voidedBy = currentUser, voidReason = "Draft deleted by scheduler" | ||
| */ | ||
| void deleteAllDrafts(); | ||
|
|
||
| /** | ||
| * Retrieve all non-voided, unsaved drafts for a user, ordered newest first. | ||
| * Drafts where markedAsSaved is true are excluded. | ||
| * | ||
| * @param userId the OpenMRS user ID (provider) | ||
| * @return list of FormDraft objects, ordered by COALESCE(dateChanged, dateCreated) DESC | ||
| */ | ||
| List<FormDraft> getAllByUserOrderedByDateDesc(Integer userId); | ||
|
|
||
| /** | ||
| * Permanently delete (hard delete) all form drafts older than the specified number of days. | ||
| * | ||
| * @param retentionDays the number of days to retain drafts | ||
| * @return the number of draft records deleted | ||
| */ | ||
| Integer deleteDraftsOlderThanDays(Integer retentionDays); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| package org.bahmni.module.bahmnicore.dao.impl; | ||
|
|
||
| import org.bahmni.module.bahmnicore.dao.FormDraftDAO; | ||
| import org.bahmni.module.bahmnicore.model.FormDraft; | ||
| import org.hibernate.SessionFactory; | ||
| import org.hibernate.query.Query; | ||
| import org.openmrs.api.context.Context; | ||
| import org.openmrs.api.db.DAOException; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.util.Calendar; | ||
| import java.util.Date; | ||
| import java.util.List; | ||
|
|
||
| public class FormDraftDaoImpl implements FormDraftDAO { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(FormDraftDaoImpl.class); | ||
|
|
||
| private SessionFactory sessionFactory; | ||
|
|
||
| public void setSessionFactory(SessionFactory sessionFactory) { | ||
| this.sessionFactory = sessionFactory; | ||
| } | ||
|
|
||
| @Override | ||
| public FormDraft saveOrUpdate(FormDraft draft) throws DAOException { | ||
| try { | ||
| sessionFactory.getCurrentSession().saveOrUpdate(draft); | ||
| return draft; | ||
| } catch (Exception e) { | ||
| log.error("Error saving or updating form draft", e); | ||
| throw new DAOException("Failed to save or update form draft", e); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public FormDraft getLatestByPatientAndUser(Integer patientId, Integer userId) throws DAOException { | ||
| try { | ||
| Query<FormDraft> query = sessionFactory.getCurrentSession() | ||
| .createQuery("FROM FormDraft WHERE patient.patientId = :patientId AND user.userId = :userId " + | ||
| "AND voided = false ORDER BY dateCreated DESC", FormDraft.class); | ||
| query.setParameter("patientId", patientId); | ||
| query.setParameter("userId", userId); | ||
| query.setMaxResults(1); | ||
| return query.uniqueResult(); | ||
| } catch (Exception e) { | ||
| log.error("Error retrieving latest form draft for patient: " + patientId + ", user: " + userId, e); | ||
| throw new DAOException("Failed to retrieve form draft", e); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void deleteLatestDraft(Integer patientId, Integer userId) throws DAOException { | ||
| try { | ||
| FormDraft draft = getLatestByPatientAndUser(patientId, userId); | ||
| if (draft != null) { | ||
| draft.setVoided(true); | ||
| draft.setDateVoided(new Date()); | ||
| draft.setVoidedBy(Context.getAuthenticatedUser()); | ||
| draft.setVoidReason("Draft deleted"); | ||
| sessionFactory.getCurrentSession().saveOrUpdate(draft); | ||
| } | ||
| } catch (DAOException e) { | ||
| throw e; | ||
| } catch (Exception e) { | ||
| log.error("Error deleting latest form draft for patient: " + patientId + ", user: " + userId, e); | ||
| throw new DAOException("Failed to delete form draft", e); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void deleteAllDrafts() throws DAOException { | ||
| try { | ||
| sessionFactory.getCurrentSession() | ||
| .createQuery("UPDATE FormDraft SET voided = true, dateVoided = :now, " + | ||
| "voidedBy = :user, voidReason = :reason WHERE voided = false") | ||
| .setParameter("now", new Date()) | ||
| .setParameter("user", Context.getAuthenticatedUser()) | ||
| .setParameter("reason", "Draft deleted by scheduler") | ||
| .executeUpdate(); | ||
| } catch (Exception e) { | ||
| log.error("Error deleting all form drafts", e); | ||
| throw new DAOException("Failed to delete all form drafts", e); | ||
| } | ||
| } | ||
|
Comment on lines
+72
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Search for file deletion logic associated with form drafts.
# Test: Look for file delete operations in the service or task layer.
rg -i "delete.*file|file.*delete|Files\.delete" bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/Repository: Bahmni/bahmni-core Length of output: 2508 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Files =="
git ls-files | rg 'FormDraft|FormDraftServiceImpl|FormDraftDaoImpl|scheduler|draft' || true
echo
echo "== FormDraftServiceImpl outline =="
ast-grep outline bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/FormDraftServiceImpl.java --view expanded || true
echo
echo "== FormDraftDaoImpl outline =="
ast-grep outline bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/dao/impl/FormDraftDaoImpl.java --view expanded || true
echo
echo "== Search for form-draft file handling =="
rg -n -i 'tempFile\.delete|FileUtils\.delete|delete.*draft|draft.*delete|voided.*file|form draft|formdraft' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/ || trueRepository: Bahmni/bahmni-core Length of output: 22914 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,260p' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/FormDraftServiceImpl.javaRepository: Bahmni/bahmni-core Length of output: 10035 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,220p' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/dao/impl/FormDraftDaoImpl.javaRepository: Bahmni/bahmni-core Length of output: 4457 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Look for any scheduled task or cleanup code around draft files.
rg -n -i 'schedule|scheduled|cron|cleanup|voided draft|deleteAllDrafts|delete.*file|tempFile\.delete|FileUtils\.delete' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/ || trueRepository: Bahmni/bahmni-core Length of output: 5522 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the FormDraft entity and service methods that touch files.
ast-grep outline bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/model/FormDraft.java --view expanded || true
printf '\n--- FormDraft entity ---\n'
sed -n '1,240p' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/model/FormDraft.java
printf '\n--- FormDraftServiceImpl ---\n'
sed -n '1,260p' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/FormDraftServiceImpl.javaRepository: Bahmni/bahmni-core Length of output: 12499 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '--- DiscardAllFormDraftsTask ---\n'
sed -n '1,220p' bahmnicore-omod/src/main/java/org/bahmni/module/bahmnicore/task/DiscardAllFormDraftsTask.java
printf '\n--- FormDraftServiceImpl discard-related methods ---\n'
sed -n '300,380p' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/FormDraftServiceImpl.java
printf '\n--- FormDraftService interface docs around scheduled task ---\n'
sed -n '1,120p' bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/FormDraftService.javaRepository: Bahmni/bahmni-core Length of output: 247 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/FormDraftServiceImpl.java')
text = p.read_text()
for needle in ['public void deleteAllDrafts', 'public void deleteDraft', 'public void deleteLatestDraft', 'public void markDraftAsSaved']:
idx = text.find(needle)
if idx != -1:
start = text.rfind('\n', 0, idx-1)
print(f'### {needle}\n')
print(text[idx:text.find('\n `@Override`', idx+1) if text.find('\n `@Override`', idx+1) != -1 else len(text)])
print('\n')
PYRepository: Bahmni/bahmni-core Length of output: 1798 Clean up draft files when voiding drafts. 🧰 Tools🪛 ast-grep (0.44.1)[warning] 73-75: SQL injection in Hibernate (sql-injection-hibernate) 🤖 Prompt for AI Agents |
||
|
|
||
| @Override | ||
| public List<FormDraft> getAllByUserOrderedByDateDesc(Integer userId) throws DAOException { | ||
| try { | ||
| Query<FormDraft> query = sessionFactory.getCurrentSession() | ||
| .createQuery("FROM FormDraft WHERE user.userId = :userId " + | ||
| "AND voided = false " + | ||
| "AND (markedAsSaved IS NULL OR markedAsSaved = false) " + | ||
| "ORDER BY COALESCE(dateChanged, dateCreated) DESC", FormDraft.class); | ||
| query.setParameter("userId", userId); | ||
| return query.getResultList(); | ||
| } catch (Exception e) { | ||
| log.error("Error retrieving all form drafts for user: " + userId, e); | ||
| throw new DAOException("Failed to retrieve form drafts for user", e); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Integer deleteDraftsOlderThanDays(Integer retentionDays) throws DAOException { | ||
| try { | ||
| Calendar calendar = Calendar.getInstance(); | ||
| calendar.add(Calendar.DAY_OF_MONTH, -retentionDays); | ||
| Date cutoffDate = calendar.getTime(); | ||
|
|
||
| Integer deletedCount = sessionFactory.getCurrentSession() | ||
| .createQuery("DELETE FROM FormDraft WHERE dateCreated < :cutoffDate") | ||
| .setParameter("cutoffDate", cutoffDate) | ||
| .executeUpdate(); | ||
| log.info("Deleted {} form drafts older than {} days", deletedCount, retentionDays); | ||
| return deletedCount; | ||
| } catch (Exception e) { | ||
| log.error("Error deleting form drafts older than {} days", retentionDays, e); | ||
| throw new DAOException("Failed to delete form drafts", e); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Exclude explicitly saved drafts from scheduled deletion.
The bulk update indiscriminately voids all non-voided drafts. Based on the logic in
getAllByUserOrderedByDateDesc(line 93), the system distinguishes between auto-saved/abandoned drafts and explicitly saved drafts via themarkedAsSavedflag. The current query will unexpectedly wipe out user-saved drafts every time the scheduled cleanup task runs.Consider adding the same filter to preserve saved drafts.
💡 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents