Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add custom form_post response writer #731

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

levkohimins
Copy link

@levkohimins levkohimins commented Jan 1, 2023

The current interface FormPostHTMLTemplateProvider only allows rendering a custom form_post template. But there is no way to pass extra template variables to be used in the template.
This pull request adds the ability to assign a custom form_post response writer.

Related Issue or Design Document

Implements a new feature, issue reference #730

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

I couldn't manage to pass nancy/test, it claims

pkg:golang/github.com/containerd/containerd@v1.4.3
7 known vulnerabilities affecting installed version

but I can't figure out which package uses this dependency.

@CLAassistant
Copy link

CLAassistant commented Jan 1, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ levkohimins
❌ Levko Burburas


Levko Burburas seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@levkohimins
Copy link
Author

@aeneasr
Hey guys, is there any chance this PR will be reviewed?

@aeneasr
Copy link
Member

aeneasr commented Jan 10, 2023

Hey, open to get this merged - there are just more things that have a higher priority on my list right now :( Was this closed on purpose?

@james-d-elliott
Copy link
Contributor

I suspect maybe because of bc43abc since his may have conflicted and he's going to open another PR. Main reason I opened that was to avoid potential conflicts down the line with unrelated PRs.

@levkohimins
Copy link
Author

Hey, open to get this merged - there are just more things that have a higher priority on my list right now

Oh, I thought that this PR went unnoticed.

@levkohimins levkohimins reopened this Jan 11, 2023
@mitar
Copy link
Contributor

mitar commented Feb 15, 2024

Please rebase this PR. Currently it contains extra commits/changes.

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.

5 participants