Skip to content

Conversation

@rajpatel24
Copy link
Contributor

@rajpatel24 rajpatel24 commented Jun 3, 2025

Summary

Set {field}_URL to 'Deleted' for deleted attachments, when include_media_url is True.

Description

  • Updated the logic to set {field}_URL to 'Deleted' if the attachment is marked as deleted and include_media_url is True.
  • Updated the test fixture to include submissions with deleted attachments and extended unit tests to verify the correct behavior of {field}_URL field.

@rajpatel24 rajpatel24 requested a review from noliveleger June 3, 2025 12:52
@rajpatel24 rajpatel24 self-assigned this Jun 3, 2025
@rajpatel24 rajpatel24 changed the title feat: Add is_deleted field and set url to Deleted for deleted attachments feat: Add {field}_is_deleted column and set {field}_URL to Deleted for deleted attachments Jun 3, 2025
class MediaField(TextField):
def get_labels(self, include_media_url=False, *args, **kwargs):
label = self._get_label(*args, **kwargs)
labels = [label, f'{label}_is_deleted']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, requirements say:

Set URL cell to ‘Deleted’ - Only if “Include media URLS” is checked

So no need to add another field. Just replace the attachment URL with "Deleted"

):
if val is None:
val = ''
first_attachment = attachment[0] if attachment else {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first_attachment is misleading IMO. I was thinking about other attachments in the same list.
What about using attachment again? attachment = attachment[0] ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! I've updated it accordingly

@rajpatel24 rajpatel24 force-pushed the handle-deleted-attachments branch from 0bca3fa to 7f6e8e4 Compare June 6, 2025 05:23
@rajpatel24 rajpatel24 changed the title feat: Add {field}_is_deleted column and set {field}_URL to Deleted for deleted attachments feat: Set {field}_URL to Deleted for attachments marked as deleted Jun 6, 2025
@coveralls
Copy link

Coverage Status

coverage: 86.674% (+0.01%) from 86.66%
when pulling 7f6e8e4 on handle-deleted-attachments
into 8735cd8 on main.

@rajpatel24 rajpatel24 requested a review from noliveleger June 6, 2025 06:28
Copy link
Contributor

@noliveleger noliveleger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rajpatel24 rajpatel24 merged commit efddba5 into main Jun 6, 2025
1 check passed
@rajpatel24 rajpatel24 deleted the handle-deleted-attachments branch June 6, 2025 15:29
rajpatel24 added a commit to kobotoolbox/kpi that referenced this pull request Jun 9, 2025
…ts TASK-575 (#5819)

### 📣 Summary
Ensure KPI installs the latest formpack version to support new export
features for deleted media attachments.



### 📖 Description
This PR updates the formpack dependency hash in `requirements.in`,
`requirements.txt`, and `dev_requirements.txt` to use the commit from
[formpack#335](kobotoolbox/formpack#335). That
PR introduces support for displaying deleted attachments more clearly in
exports:

- Replaces `{field}_URL` with the text `Deleted` when the attachment is
marked as deleted and `include_media_url=True`.

These changes ensure that exports generated by KPI reflect deleted
attachments more transparently for end users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants