Skip to content

Enable customizing login/logout responses - #296

Merged
giovannicimolin merged 2 commits into
jazzband:developfrom
c-w:customize-logout-response
Oct 17, 2023
Merged

Enable customizing login/logout responses#296
giovannicimolin merged 2 commits into
jazzband:developfrom
c-w:customize-logout-response

Conversation

@c-w

@c-w c-w commented Jan 16, 2023

Copy link
Copy Markdown
Contributor

This pull request is a small change to enable customizing the response shape for the login and logout views by overriding a simple method.

This is useful in situations where for example the 204 responses in the logout views are undesirable (e.g. for clients that always assume that responses have a JSON body).

@AdamDonna

Copy link
Copy Markdown
Contributor

This looks very reasonable to me 👍
I think you're good to merge if I can get another +1.
@giovannicimolin does this look good to you?

@giovannicimolin

Copy link
Copy Markdown
Contributor

@c-w Thanks for the contribution! 😁

I'll be reviewing these changes in the next week or so.
I don't have a lot of bandwidth but I'll try to keep things moving.

CC @AdamDonna

@johnraz

johnraz commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

This LGTM as well, maybe worth adding to the documentation though.

@c-w

c-w commented Oct 16, 2023

Copy link
Copy Markdown
Contributor Author

This LGTM as well, maybe worth adding to the documentation though.

Where would you like me to document this @johnraz ?

@johnraz

johnraz commented Oct 16, 2023

Copy link
Copy Markdown
Contributor

@giovannicimolin giovannicimolin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

LGTM, changes look good and there's some interest in this being merged.

I think this is almost good to merge, but I agree with @johnraz that it needs some documentation. If you add those docs in, I'll make sure to review and move this along. 😁

@c-w

c-w commented Oct 17, 2023

Copy link
Copy Markdown
Contributor Author

it needs some documentation

I added the docs in 43d983d. PTAL @giovannicimolin

@c-w
c-w requested a review from giovannicimolin October 17, 2023 08:09
@codecov

codecov Bot commented Oct 17, 2023

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (develop@3a1bc58). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop     #296   +/-   ##
==========================================
  Coverage           ?   91.70%           
==========================================
  Files              ?        9           
  Lines              ?      229           
  Branches           ?       35           
==========================================
  Hits               ?      210           
  Misses             ?       16           
  Partials           ?        3           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@giovannicimolin

Copy link
Copy Markdown
Contributor

👍

CI checks are green, we're good to go.
@c-w Thanks for your contribution. 😁

@giovannicimolin
giovannicimolin merged commit ffd9171 into jazzband:develop Oct 17, 2023
@c-w
c-w deleted the customize-logout-response branch October 17, 2023 13:14
dontexit pushed a commit to dontexit/django-rest-knox that referenced this pull request Jan 24, 2024
Enable customizing login/logout responses
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