You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/05/20 09:35:00 UTC
[GitHub] [fineract] ptuomola opened a new pull request #914: FINERACT-723: Fixing date comparisons
ptuomola opened a new pull request #914:
URL: https://github.com/apache/fineract/pull/914
## Description
Three integration tests were using the local server timezone rather than using the correct utility function to retrieve the tenant timezone in test
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm has been updated with details of any API changes.
- [ ] Integration tests have been created/updated for verifying the changes made.
- [ ] All Integrations tests are passing with the new commits.
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger merged pull request #914: FINERACT-723: Fixing date comparisons
Posted by GitBox <gi...@apache.org>.
vorburger merged pull request #914:
URL: https://github.com/apache/fineract/pull/914
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #914: FINERACT-723: Fixing date comparisons
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #914:
URL: https://github.com/apache/fineract/pull/914#issuecomment-631414878
See https://github.com/apache/fineract/pull/627 do we need to remove the hard coded Timezone we are setting in Travis now with this PR? @vorburger ..Maybe?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #914: FINERACT-723: Fixing date comparisons
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #914:
URL: https://github.com/apache/fineract/pull/914#issuecomment-631853470
> See #627 do we need to remove the hard coded Timezone we are setting in Travis now with this PR? @vorburger ..Maybe?
The Utils class used by RestAssured also has the tenant timezone hardcoded:
in org.apache.fineract.integrationtests.common.Utils:
public static final String TENANT_TIME_ZONE = "Asia/Kolkata";
A better solution would of course be to change the Utils class to look this up from the database.
After my PR, the current IT scope seems to work as everything is hardcoded to point to the same timezone (Asia/Kolkata). To make it work with any timezone for tenant/database/application server without any hardcoding, then we will need to review/update all code and queries using either application server date and database date.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #914: FINERACT-723: Fixing date comparisons
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #914:
URL: https://github.com/apache/fineract/pull/914#issuecomment-631851215
> This looks good to me.
> In https://issues.apache.org/jira/browse/FINERACT-723 , Seems there are other cases like this in other parts of the code base? Do we want to look at that later? in a separate issue?
You're right: based on the JIRA, there are probably other scenarios that also lead to failures. However these are the only test that fail on my machine every day if I run the integration tests in the evening - so I had a personal interest in fixing them. My suggestion would be to incorporate this PR if it looks OK to you, but we can still keep the JIRA open for other fixes of the same nature.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola edited a comment on pull request #914: FINERACT-723: Fixing date comparisons
Posted by GitBox <gi...@apache.org>.
ptuomola edited a comment on pull request #914:
URL: https://github.com/apache/fineract/pull/914#issuecomment-631853470
> See #627 do we need to remove the hard coded Timezone we are setting in Travis now with this PR? @vorburger ..Maybe?
The Utils class used by RestAssured also has the tenant timezone hardcoded:
in org.apache.fineract.integrationtests.common.Utils:
public static final String TENANT_TIME_ZONE = "Asia/Kolkata";
A better solution would of course be to change the Utils class to look this up from the database.
After my PR, the current IT scope seems to work as everything is hardcoded to point to the same timezone (Asia/Kolkata).
To make everything work with any timezone for tenant/database/application server without any hardcoding, then we will need to review/update all code and queries using either application server date and database date.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org