ptuomola commented on a change in pull request #1290:
URL: https://github.com/apache/fineract/pull/1290#discussion_r480686680
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/organisation/CampaignsHelper.java
##########
@@ -53,50 +53,50 @@ public CampaignsHelper(final RequestSpecification
requestSpec, final ResponseSpe
public Integer createCampaign(String reportName, Integer triggerType) {
LOG.info("---------------------------------CREATING A
CAMPAIGN---------------------------------------------");
- final String CREATE_SMS_CAMPAIGNS_URL = SMS_CAMPAIGNS_URL + "?" +
Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec,
CREATE_SMS_CAMPAIGNS_URL, getCreateCampaignJSON(reportName, triggerType),
+ final String createSMScampaignsURL = SMS_CAMPAIGNS_URL + "?" +
Utils.TENANT_IDENTIFIER;
Review comment:
We've made SMS lowercase elsewhere, so would these make more sense as
createSmsCampaignsURL etc?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/EmbeddedTomcatWithSSLConfiguration.java
##########
@@ -55,7 +55,7 @@ protected Connector createSslConnector() {
File keystore = getFile(getKeystore());
connector.setScheme("https");
connector.setSecure(true);
- connector.setPort(getHTTPSPort());
+ connector.setPort(getHTTPSport());
Review comment:
Would getHttpsPort not make more sense - now it looks like getHttpSport
:-)
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/SchedulerJobHelper.java
##########
@@ -76,32 +76,32 @@ public SchedulerJobHelper(final RequestSpecification
requestSpec) {
}
public Map<String, Object> getSchedulerJobById(int jobId) {
- final String GET_SCHEDULER_JOB_BY_ID_URL =
"/fineract-provider/api/v1/jobs/" + jobId + "?" + Utils.TENANT_IDENTIFIER;
+ final String getSchedularJobBYIDurl =
"/fineract-provider/api/v1/jobs/" + jobId + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING SCHEDULER JOB BY ID
-------------------------");
Review comment:
I think this should be getSchedulerJobByIdURL
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/LoanWithWaiveInterestAndWriteOffIntegrationTest.java
##########
@@ -140,7 +140,7 @@ public void checkClientLoanCreateAndDisburseFlow() {
}
@Test
- public void checkClientLoan_WRITTEN_OFF() {
+ public void checkClientLoanWrittenOFF() {
Review comment:
I don't think OFF is an abbreviation - so if we use CamelCase, let's do
it for the whole word (even if CheckStyle would allow us not to)
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/LoanWithWaiveInterestAndWriteOffIntegrationTest.java
##########
@@ -140,7 +140,7 @@ public void checkClientLoanCreateAndDisburseFlow() {
}
@Test
- public void checkClientLoan_WRITTEN_OFF() {
+ public void checkClientLoanWrittenOFF() {
// CREATE CLIENT
Review comment:
OFF is not a constant so shouldn't this be checkClientLoanWrittenOff
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/GroupSavingsIntegrationTest.java
##########
@@ -160,10 +160,10 @@ public void testSavingsAccount_CLOSE_APPLICATION() {
DateFormat dateFormat = new SimpleDateFormat("dd MMMM yyyy",
Locale.US);
Calendar todaysDate = Calendar.getInstance();
- final String CLOSEDON_DATE = dateFormat.format(todaysDate.getTime());
+ final String closedOnDateValue =
dateFormat.format(todaysDate.getTime());
String withdrawBalance = "false";
Review comment:
Is there a reason for adding ...Value for this specifically?
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositAccountHelper.java
##########
@@ -156,9 +156,9 @@ public static Integer
applyRecurringDepositApplication(final String recurringDep
public static HashMap getRecurringDepositAccountById(final
RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_RECURRING_DEPOSIT_BY_ID_URL =
RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getRecurringDepositByIDurl =
RECURRING_DEPOSIT_ACCOUNT_URL + "/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
Would getRecurringDepositByIdURL be more consistent with the others that
capitalise URL?
##########
File path:
fineract-provider/src/main/java/org/apache/fineract/portfolio/group/api/GroupsApiResource.java
##########
@@ -624,11 +624,11 @@ public String retrieveGsimAccounts(@PathParam("groupId")
final Long groupId,
}
- final Set<String> GSIM_ACCOUNTS_DATA_PARAMETERS = new HashSet<>(
+ final Set<String> glimAccountsDataParameters = new HashSet<>(
Arrays.asList("gsimId", "groupId", "accountNumber",
"childGSIMAccounts", "parentBalance", "savingsStatus"));
Review comment:
Mistake here I think - shouldn't this be gsim not glim - as in the
original code?
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/recurringdeposit/RecurringDepositProductHelper.java
##########
@@ -244,8 +244,8 @@ public static ArrayList
retrieveAllRecurringDepositProducts(final RequestSpecifi
public static HashMap retrieveRecurringDepositProductById(final
RequestSpecification requestSpec,
final ResponseSpecification responseSpec, final String productId) {
LOG.info("-------------------- RETRIEVING RECURRING DEPOSIT PRODUCT BY
ID --------------------------");
- final String GET_RD_PRODUCT_BY_ID_URL = RECURRING_DEPOSIT_PRODUCT_URL
+ "/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec,
responseSpec, GET_RD_PRODUCT_BY_ID_URL, "");
+ final String getRDproductByIDurl = RECURRING_DEPOSIT_PRODUCT_URL + "/"
+ productId + "?" + Utils.TENANT_IDENTIFIER;
Review comment:
Would getRdProductByIdURL be more consistent with the others?
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/HolidayHelper.java
##########
@@ -77,15 +77,15 @@ public static Integer createHolidays(final
RequestSpecification requestSpec, fin
public static Integer activateHolidays(final RequestSpecification
requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String ACTIVATE_HOLIDAY_URL = HOLIDAYS_URL + "/" + holidayID +
"?command=activate&" + Utils.TENANT_IDENTIFIER;
- return Utils.performServerPost(requestSpec, responseSpec,
ACTIVATE_HOLIDAY_URL, getActivateHolidayDataAsJSON(), "resourceId");
+ final String activateHolidayURL = HOLIDAYS_URL + "/" + holidayID +
"?command=activate&" + Utils.TENANT_IDENTIFIER;
+ return Utils.performServerPost(requestSpec, responseSpec,
activateHolidayURL, getActivateHolidayDataAsJSON(), "resourceId");
}
public static HashMap getHolidayById(final RequestSpecification
requestSpec, final ResponseSpecification responseSpec,
final String holidayID) {
- final String GET_HOLIDAY_BY_ID_URL = HOLIDAYS_URL + "/" + holidayID +
"?" + Utils.TENANT_IDENTIFIER;
+ final String getHolidayByIDurl = HOLIDAYS_URL + "/" + holidayID + "?"
+ Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING HOLIDAY BY ID
-------------------------");
Review comment:
Would be good to be consistent with capitalising URL - so this would be
getHolidayByIdURL
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/loans/LoanTransactionHelper.java
##########
@@ -303,46 +303,46 @@ public HashMap withdrawLoanApplicationByClient(final
String date, final Integer
public Integer addChargesForLoan(final Integer loanId, final String
request) {
LOG.info("--------------------------------- ADD CHARGES FOR LOAN
--------------------------------");
- final String ADD_CHARGES_URL = "/fineract-provider/api/v1/loans/" +
loanId + "/charges?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerPost(requestSpec,
responseSpec, ADD_CHARGES_URL, request, "");
+ final String chargesURL = "/fineract-provider/api/v1/loans/" + loanId
+ "/charges?" + Utils.TENANT_IDENTIFIER;
Review comment:
Is there a reason for dropping the add from the name? Why not
addChargesURL?
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositProductHelper.java
##########
@@ -436,8 +436,8 @@ public static ArrayList
retrieveAllFixedDepositProducts(final RequestSpecificati
public static HashMap retrieveFixedDepositProductById(final
RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String productId) {
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT PRODUCT BY
ID ------------------------");
- final String GET_FD_PRODUCT_BY_ID_URL = FIXED_DEPOSIT_PRODUCT_URL +
"/" + productId + "?" + Utils.TENANT_IDENTIFIER;
- final HashMap response = Utils.performServerGet(requestSpec,
responseSpec, GET_FD_PRODUCT_BY_ID_URL, "");
+ final String getFDproductByIDurl = FIXED_DEPOSIT_PRODUCT_URL + "/" +
productId + "?" + Utils.TENANT_IDENTIFIER;
+ final HashMap response = Utils.performServerGet(requestSpec,
responseSpec, getFDproductByIDurl, "");
Review comment:
I think cleaner and more consistent would be getFdProductByIdURL - what
do you think?
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/fixeddeposit/FixedDepositAccountHelper.java
##########
@@ -144,9 +144,9 @@ public static Integer applyFixedDepositApplication(final
String fixedDepositAcco
public static HashMap getFixedDepositAccountById(final
RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final Integer accountID) {
- final String GET_FIXED_DEPOSIT_BY_ID_URL = FIXED_DEPOSIT_ACCOUNT_URL +
"/" + accountID + "?" + Utils.TENANT_IDENTIFIER;
+ final String getFixedDepositByIDurl = FIXED_DEPOSIT_ACCOUNT_URL + "/"
+ accountID + "?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING FIXED DEPOSIT ACCOUNT BY
ID -------------------------");
Review comment:
Would be good to be consistent in capitalising URL - so this would be
getFixedDepositByIdURL
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -65,7 +65,7 @@
* Client Loan Integration Test for checking Loan Application Repayments
Schedule, loan charges, penalties, loan
* repayments and verifying accounting transactions
*/
-@SuppressWarnings({ "rawtypes", "unchecked" })
+@SuppressWarnings({ "rawtypes", "unchecked", "AbbreviationAsWordInName" })
public class ClientLoanIntegrationTest {
Review comment:
What was the reason for suppressing for this file?
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/StandingInstructionsHelper.java
##########
@@ -108,20 +108,20 @@ public Integer createStandingInstruction(final String
clientId, final String fro
public HashMap getStandingInstructionById(final String
standingInstructionId) {
LOG.info("----------------------------- RETRIEVING STANDING
INSTRUCTION BY ID---------------------------");
- final String GET_STANDING_INSTRUCTION_BY_ID_URL =
STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ final String getStandingInstructionsByIDurl =
STANDING_INSTRUCTIONS_URL + "/" + standingInstructionId + "?"
+ Utils.TENANT_IDENTIFIER;
Review comment:
Would be good to be consistent in capitalising URL - so this would be
getStandingInstructionsByIdURL
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/MinimumDaysBetweenDisbursalAndFirstRepaymentTest.java
##########
@@ -43,7 +43,7 @@
/**
* Test the creation, approval and rejection of a loan reschedule request
**/
-@SuppressWarnings({ "rawtypes" })
+@SuppressWarnings({ "rawtypes", "AbbreviationAsWordInName" })
public class MinimumDaysBetweenDisbursalAndFirstRepaymentTest {
Review comment:
What was the reason for suppressing for this file? I didn't see anything
specific why we would not be able to use... but perhaps I missed something
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/common/GlobalConfigurationHelper.java
##########
@@ -41,18 +41,17 @@ public GlobalConfigurationHelper(final RequestSpecification
requestSpec, final R
public static ArrayList<HashMap> getAllGlobalConfigurations(final
RequestSpecification requestSpec,
final ResponseSpecification responseSpec) {
- final String GET_ALL_GLOBAL_CONFIG_URL =
"/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
+ final String getAllGlobalConfigURL =
"/fineract-provider/api/v1/configurations?" + Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING ALL GLOBAL
CONFIGURATIONS -------------------------");
- final HashMap<String, ArrayList<HashMap>> response =
Utils.performServerGet(requestSpec, responseSpec, GET_ALL_GLOBAL_CONFIG_URL,
- "");
+ final HashMap<String, ArrayList<HashMap>> response =
Utils.performServerGet(requestSpec, responseSpec, getAllGlobalConfigURL, "");
return response.get("globalConfiguration");
}
public static HashMap getGlobalConfigurationById(final
RequestSpecification requestSpec, final ResponseSpecification responseSpec,
final String configId) {
- final String GET_GLOBAL_CONFIG_BY_ID_URL =
"/fineract-provider/api/v1/configurations/" + configId + "?" +
Utils.TENANT_IDENTIFIER;
+ final String getGlobalConfigByIDurl =
"/fineract-provider/api/v1/configurations/" + configId + "?" +
Utils.TENANT_IDENTIFIER;
LOG.info("------------------------ RETRIEVING GLOBAL CONFIGURATION BY
ID -------------------------");
Review comment:
Would be good to be consistent at capitalising URL - so this would be
getGlobalConfigByIdURL
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/SchedulerJobsTestResults.java
##########
@@ -537,11 +537,11 @@ public void testUpdateLoanSummaryJobOutcome() throws
InterruptedException {
final String currentDate = dateFormat.format(todaysDate.getTime());
todaysDate.add(Calendar.MONTH, -1);
- final String LOAN_DISBURSEMENT_DATE =
dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue =
dateFormat.format(todaysDate.getTime());
Review comment:
Is there a reason why we have ...Value added to some (but not all) of
these?
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientLoanIntegrationTest.java
##########
@@ -3117,7 +3117,7 @@ public void
testLoanScheduleWithInterestRecalculation_WITH_REST_SAME_AS_REPAYMEN
Calendar todaysDate =
Calendar.getInstance(Utils.getTimeZoneOfTenant());
todaysDate.add(Calendar.DAY_OF_MONTH, -14);
- final String LOAN_DISBURSEMENT_DATE =
dateFormat.format(todaysDate.getTime());
+ final String loanDisbursementDateValue =
dateFormat.format(todaysDate.getTime());
Review comment:
This one has ...Value, others don't. Is there a difference that you had
in mind?
----------------------------------------------------------------
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:
us...@infra.apache.org
