Skip to content

Conversation

@abhi1693
Copy link
Member

Fixes: #12538

I have added support for parsing return_url from the request in the table if it exists. The old functionality is still retained if the parameter is not provided. I could not find any other clean way of redirecting the user back to where the request was initiated.

@abhi1693 abhi1693 requested a review from jeremystretch May 26, 2023 10:37
Comment on lines 238 to 239
query_params = request.GET.copy() if request else {}
if return_url := query_params.pop('return_url', None):
Copy link
Member

Choose a reason for hiding this comment

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

I could be missing something but this would this not be sufficient?

Suggested change
query_params = request.GET.copy() if request else {}
if return_url := query_params.pop('return_url', None):
if return_url := request.GET.get('return_url'):

Copy link
Member

Choose a reason for hiding this comment

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

We can probably simplify this to something like:

return_url = request.GET.get('return_url', request.get_full_path())
url_appendix = f'?return_url={quote(return_url)}' if return_url else ''

Copy link
Member Author

Choose a reason for hiding this comment

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

I could be missing something but this would this not be sufficient?

I tested this using services on device and that worked just it was before. Then did the same test using image attachment and post edit and delete, I landed up on the device page.

While implementing this, I was thinking the same solution should be implemented application wide though.

If you can think of a use-case, that I ay have missed let know.

Copy link
Member

Choose a reason for hiding this comment

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

I've introduced the htmx_table template tag as a utility for consistently embedding HTMX tables in this manner. As employing its use beyond image attachments is outside the scope of this issue, I'm going to consider this fixed, and open a separate issue to track its further adoption.

@jeremystretch jeremystretch merged commit dbd3c6d into develop May 31, 2023
@jeremystretch jeremystretch deleted the fix/12538-table-return-url branch May 31, 2023 19:22
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete image attachment on device (type) should return to device not to image attachments list

3 participants