django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

Use POST method instead of GET to perform logout in browsable API

Open realsuayip opened this issue 1 year ago • 0 comments

Description

Fixes #9206

realsuayip avatar Jan 03 '24 20:01 realsuayip

@elhussienalmasri - I have prepared this issue with a bit more detail and some other ideas on approaches.

Hopefully you are up for continuing on this solution, please add comments & questions here if you get stuck.

lb- avatar Feb 05 '24 00:02 lb-

yes @lb- , I am very interested to continue to work on this issue , and already follow your last comment , and I will take a look on this valuable info also. I thought pass choosers urls from Backend to Frontend easy but I found it require some concepts and I face some problems , and I already solved most of it and may be I can raise PR this week if I can ,t make PR at least I will tell my update on work on this issue , I will try to find time as possible to continue work and finish this issue.

elhussienalmasri avatar Feb 05 '24 04:02 elhussienalmasri

Thanks for the update.

lb- avatar Feb 05 '24 06:02 lb-

From @gasman - https://github.com/wagtail/wagtail/pull/11571#issuecomment-1927950540

I wouldn't be opposed to moving the contents of _editor_js.html into the base template - along with the appropriate cleanup (such as eliminating places where pages import things like modal-workflow.js individually, which will now be duplicated) and deprecating the insert_editor_js hook.

I'd like us to continue working towards reducing our reliance on globally-defined JS, especially when it comes to our Draftail widget. Moving these JS imports from a "sort-of-global" include to an "actually global" one might seem like a step backwards - but the whole concept of "the page editor" as a specific part of the admin that requires a bunch of extra JS is quite an outdated one, now that we're moving more and more of the editing interface into generic views. Ideally, we would use form media to ensure that the supporting scripts for Draftail only get brought in when needed, but until we reach that goal, I think it's better to have them just work everywhere, rather than requiring pages to pretend to be "the page editor" to work correctly.


Additional notes.

  • This makes sense, we will continue with the attempt to remove the window.chooserUrls under this issue.
  • Once resolved, we may try to close out https://github.com/wagtail/wagtail/issues/2936 by moving the editor_js.html content to the base admin template.
  • There's also the potential of us deprecating the insert_editor_js hook, but that can be addressed separately if needed in the future.

lb- avatar Feb 05 '24 23:02 lb-

@elhussienalmasri I have been playing around with your PR and reviewing some other ideas, I think we can do this with the following changes.

A. Pass in the chooserUrls as an options object when we register the entity.

This is a different approach to your PR where you are adding all the options in wagtail/admin/rich_text/editors/draftail/__init__.py even for apps that may not be used.

Firstly, we get an object that is for the general link chooser.

# wagtail/admin/wagtail_hooks.py
from django.urls import reverse, reverse_lazy # add reverse_lazy
# other imports

@hooks.register("register_rich_text_features")
def register_core_features(features):
   # ... all the other things
    features.register_editor_plugin(
        "draftail",
        "link",
        draftail_features.EntityFeature(
            {
                "type": "LINK",
                "icon": "link",
                "description": gettext("Link"),
                # We want to enforce constraints on which links can be pasted into rich text.
                # Keep only the attributes Wagtail needs.
                "attributes": ["url", "id", "parentId"],
                "allowlist": {
                    # Keep pasted links with http/https protocol, and not-pasted links (href = undefined).
                    "href": "^(http:|https:|undefined$)",
                },
                "chooserUrls": { # add this object here, essentially the same as what's in editor_js.html but not in the template, using reverse_lazy
                    'pageChooser': reverse_lazy("wagtailadmin_choose_page"),
                    'externalLinkChooser': reverse_lazy("wagtailadmin_choose_page_external_link"),
                    'emailLinkChooser': reverse_lazy("wagtailadmin_choose_page_email_link"),
                    'phoneLinkChooser': reverse_lazy("wagtailadmin_choose_page_phone_link"),
                    'anchorLinkChooser': reverse_lazy("wagtailadmin_choose_page_anchor_link"),
                }
            },
            js=[
                "wagtailadmin/js/page-chooser-modal.js",
            ],
        ),
    )

Now for the documents app, we do a similar change but in the document hooks only.

# wagtail/documents/wagtail_hooks.py

from django.urls import include, path, reverse, reverse_lazy

# ... other stuff

@hooks.register("register_rich_text_features")
def register_document_feature(features):
    features.register_link_type(DocumentLinkHandler)

    features.register_editor_plugin(
        "draftail",
        "document-link",
        draftail_features.EntityFeature(
            {
                "type": "DOCUMENT",
                "icon": "doc-full-inverse",
                "description": gettext("Document"),
                "chooserUrls": { # add this object structure, using reverse_lazy
                    "documentChooser": reverse_lazy("wagtaildocs_chooser:choose")
                }
            },
            js=["wagtaildocs/js/document-chooser-modal.js"],
        ),
    )

B. On the Draftail client-side code

In each of the getChooserConfig methods, create a new object that is a merge of the global and the props entityType chooser Urls as follows.

Note: Embed/Image need to be done but rough examples below.

class LinkModalWorkflowSource extends ModalWorkflowSource {
  getChooserConfig(entity, selectedText) {
    const chooserUrls = {
      ...this.props.entityType?.chooserUrls,
      ...global.chooserUrls,
    };

    let url = chooserUrls.pageChooser;

    const urlParams = {
      page_type: 'wagtailcore.page',
      allow_external_link: true,
      allow_email_link: true,
      allow_phone_link: true,
      allow_anchor_link: true,
      link_text: selectedText,
    };

    if (entity) {
      const data = entity.getData();

      if (data.id) {
        if (data.parentId !== null) {
          url = `${chooserUrls.pageChooser}${data.parentId}/`;
        } else {
          url = chooserUrls.pageChooser;
        }
      } else if (data.url.startsWith('mailto:')) {
        url = chooserUrls.emailLinkChooser;
        urlParams.link_url = data.url.replace('mailto:', '');
      } else if (data.url.startsWith('tel:')) {
        url = chooserUrls.phoneLinkChooser;
        urlParams.link_url = data.url.replace('tel:', '');
      } else if (data.url.startsWith('#')) {
        url = chooserUrls.anchorLinkChooser;
        urlParams.link_url = data.url.replace('#', '');
      } else {
        url = chooserUrls.externalLinkChooser;
        urlParams.link_url = data.url;
      }
    }
// ...
class DocumentModalWorkflowSource extends ModalWorkflowSource {
  getChooserConfig() {
    const { documentChooser } = {
      ...this.props.entityType?.chooserUrls,
      ...global.chooserUrls,
    };

    return {
      url: documentChooser,
      urlParams: {},
      onload: global.DOCUMENT_CHOOSER_MODAL_ONLOAD_HANDLERS,
      responses: {
        chosen: this.onChosen,
      },
    };
  }
// ... 

Now, this only creates a sub-set of chooserUrls for each ModalWorkflowSource (i.e. the DocumentModalWorkflowSource would not be able to use the image chooser URL or the page chooser URL). However, I think this is fine from my understanding of the code. client/src/components/Draftail/sources/ModalWorkflowSource.js - it looks looks like each ModalWorkflowSource does not need to access chooser URls outside of its own group.

I did some testing and this worked well.

Note that you will not be able to print the output of the reverse lazy URLs for debugging on the server side (mostly) as they have to resolve late.

The lazy URLs will resolve when json.dumps runs in wagtail/admin/rich_text/editors/draftail/__init__.py DraftailRichTextArea.get_context.

Hope this helps, your PR is on the right track but this should hopefully get you to a place where you can validate things and start writing unit tests.

lb- avatar Feb 12 '24 07:02 lb-

Great , Thanks @lb- for the detailed explanation.

elhussienalmasri avatar Feb 12 '24 20:02 elhussienalmasri