Skip to content

Issue #11 Set ClientIp via the parameters form. #15

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

judgej
Copy link
Member

@judgej judgej commented Sep 6, 2016

Still not ideal - seems too much repitition in code. Also the clientIp is
not retreived from the session, but many other fields (e.g. return URLs) are also
not retrieved from the session when re-presenting the request form, so it is
consistent at least, but could do with some improvements.

Still not ideal - seems too much repitition in code. Also the clientIp is
not retreived from the session, but many other fields (e.g. return URLs) are also
not retrieved from the session when re-presenting the request form, so it is
consistent at least, but could do with some improvements.
@judgej
Copy link
Member Author

judgej commented Sep 6, 2016

The idea of this is that for the demo purposes, the client IP address is not blindly passed to the gateway driver. Instead it is presented to the user first, who can modify it or blank it out if necessary, then submit the transaction with the modified IP.

@judgej
Copy link
Member Author

judgej commented Sep 6, 2016

I cannot see the fork this PR is based on listed anywhere in the "forks" window, so I am not entirely confident that it contains the right code. I think sometimes, when in branch hell, the only option is to burn it all down and start again. I'll do that tomorrow.

@delatbabel
Copy link
Contributor

Burning it down and starting again gets my vote. It kinda looks OK at first glance, though.

@judgej
Copy link
Member Author

judgej commented Feb 21, 2019

This is a bit old! I blink once and I'm away from the example application for three years. Still relevant, though I need to look again to make sure the solution is the right approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants