Skip to content

Bulk create HTML snapshots#1132

Merged
sissbruecker merged 7 commits into
sissbruecker:masterfrom
Tql-ws1:feat/bulk-create-html-snapshot
Aug 12, 2025
Merged

Bulk create HTML snapshots#1132
sissbruecker merged 7 commits into
sissbruecker:masterfrom
Tql-ws1:feat/bulk-create-html-snapshot

Conversation

@Tql-ws1

@Tql-ws1 Tql-ws1 commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

Closes #869

<option value="bulk_unshare">Unshare</option>
{% endif %}
<option value="bulk_refresh">Refresh from website</option>
<option value="bulk_snapshot">Create HTML snapshot</option>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is only supported in the plus image, so it would be better to only show this if the feature is actually active. Here is a check that implements this when scheduling a background task:

def is_html_snapshot_feature_active() -> bool:
return settings.LD_ENABLE_SNAPSHOTS and not settings.LD_DISABLE_BACKGROUND_TASKS

To make this available in the template I would suggest to expose settings.LD_ENABLE_SNAPSHOTS in a field in BookmarkListContext.

Would also be good to have tests to verify that the option is only shown when the feature is enabled.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing out the aspects I had overlooked. I’ve made the corresponding adjustments and added unit test cases.

Should I further refine the way settings.LD_ENABLE_SNAPSHOTS is exposed? Using a naming like bookmark_list.snapshot_feature_enabled in the bulk_edit/bar.html template feels somewhat less intuitive in conveying the meaning.

Also, do you think it would be beneficial to include end-to-end test cases as well?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Should be good enough for now

@Tql-ws1 Tql-ws1 force-pushed the feat/bulk-create-html-snapshot branch from 55983f6 to 87df770 Compare August 11, 2025 16:53
@Tql-ws1

Tql-ws1 commented Aug 11, 2025

Copy link
Copy Markdown
Contributor Author

Sorry, I recently switched from using git to jujutsu, and the force push above should have been pushed to a branch in my fork repository.

This test case covers the scenario where the bulk actions panel displays the corresponding options when the HTML snapshot feature is enabled.
@Tql-ws1 Tql-ws1 force-pushed the feat/bulk-create-html-snapshot branch from 87df770 to 148f8d2 Compare August 11, 2025 17:12
@sissbruecker sissbruecker changed the title Add option to create HTML snapshot for bulk edit Bulk create HTML snapshots Aug 12, 2025
@sissbruecker sissbruecker merged commit c746afc into sissbruecker:master Aug 12, 2025
2 checks passed
@Tql-ws1 Tql-ws1 deleted the feat/bulk-create-html-snapshot branch August 16, 2025 23:01
WooHooDai pushed a commit to WooHooDai/linkding-cn that referenced this pull request Oct 1, 2025
* Add option to create HTML snapshot for bulk edit

* Add the prerequisite for displaying the "Create HTML Snapshot" bulk action option

* Add test case

This test case covers the scenario where the bulk actions panel displays the corresponding options when the HTML snapshot feature is enabled.

* Use the existing `tasks.create_html_snapshots()` instead of the for loop

* Fix the exposure of `settings.LD_ENABLE_SNAPSHOTS` within `BookmarkListContext`

* add service tests

* cleanup context

---------

Co-authored-by: Sascha Ißbrücker <sascha.issbruecker@gmail.com>
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.

Bulk edit for "Create html snapshot"

2 participants