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

added createdAt property for issue #43 #48

Merged
merged 2 commits into from
Feb 8, 2017
Merged

added createdAt property for issue #43 #48

merged 2 commits into from
Feb 8, 2017

Conversation

netbull
Copy link
Contributor

@netbull netbull commented Feb 3, 2017

quick fix for issue #43 until the backend returns real lifetime of the token

@stefanpolzer
Copy link
Contributor

stefanpolzer commented Feb 3, 2017

@netbull I'm not sure if this is the right way e.g. in my case some response go into a queue first.

So the createdAt have nothing to do with the validity. The right naming should be parsedAt but I'm not sure if anyone needs that...

We can't fix this on the SDK site along the only good way is to get it from the Server.

@netbull
Copy link
Contributor Author

netbull commented Feb 3, 2017

@stefanpolzer it's not the right way.

this is temporary solution in order to know when the token was created and the validity check can be if it's less than X minutes, recreate the token.

The right way should be if the response returns info until when is valid /when it expires/ and the validation will be much easier and correct

I don't get it why you may need to put the response in a queue... O_O if you have many request to process in the queue and theoretically until the time to process a response the token is expired what will happen?

@stefanpolzer
Copy link
Contributor

stefanpolzer commented Feb 3, 2017

Ask a for a new one... but this more a theory...

If it's only a temporary solutions than think twice... one day there is the info send by the server will you remove it and bracket some backwards compatibility or will you keep it there forever.

BTW way do you have included a setter? How should be allowed to change the createdAt property?

@stefanpolzer
Copy link
Contributor

And because this is not really a value send by the server I would put it in the abstract class as a general value available for all responses.... make more sense to me

@netbull
Copy link
Contributor Author

netbull commented Feb 3, 2017

@stefanpolzer yes, that's better... I guess

the setter was auto generated...dumb thing..

@stefanpolzer
Copy link
Contributor

Yes this is a workaround that can stay there 😉 even if we get the real valid till value

@stefanpolzer
Copy link
Contributor

@netbull @tobiaslins one more topic to think about. Every value in all the Response Classes are primitive data types link sting, int, bool now we have a Object...

@stefanpolzer
Copy link
Contributor

stefanpolzer commented Feb 3, 2017

Usually I prefer Object, but because others may want to us it as well I would just go with the Unix Timestamp int time()
Than ever body can use the object that want's... e.g. I prefer Carbon over DateTime

@stefanpolzer
Copy link
Contributor

@tobiaslins what do you think?

@tobiaslins tobiaslins merged commit f30c38b into mpay24:master Feb 8, 2017
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.

3 participants