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 2022/07/25 08:38:59 UTC
[GitHub] [fineract] adamsaghy opened a new pull request, #2453: Auditable loan and client and loan transaction
adamsaghy opened a new pull request, #2453:
URL: https://github.com/apache/fineract/pull/2453
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are 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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2453: Auditable loan and client and loan transaction
Posted by GitBox <gi...@apache.org>.
galovics merged PR #2453:
URL: https://github.com/apache/fineract/pull/2453
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2453: Auditable loan and client and loan transaction
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2453:
URL: https://github.com/apache/fineract/pull/2453#discussion_r928878978
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -180,16 +180,14 @@ public final class Client extends AbstractPersistableCustom {
@Column(name = "submittedon_date", nullable = true)
private LocalDate submittedOnDate;
- @ManyToOne(optional = true, fetch = FetchType.LAZY)
- @JoinColumn(name = "submittedon_userid", nullable = true)
- private AppUser submittedBy;
+ // Deprecated since common Auditable fields were introduced. Columns and data left untouched to help migration.
- @Column(name = "updated_on", nullable = true)
- private LocalDate updatedOnDate;
+ // @Column(name = "updated_on", nullable = true)
Review Comment:
Can we use multi-line comment here? Sorry, I know it's a minor thing but it groups these lines well together and nobody is going to confuse it with actual comments that could be deleted. Thanks.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java:
##########
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.domain;
+
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_DATE_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_DATE_DB_FIELD;
+
+import java.time.OffsetDateTime;
+import java.util.Optional;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.springframework.data.domain.Auditable;
+import org.springframework.data.jpa.domain.AbstractAuditable;
+
+/**
+ * A custom copy of {@link AbstractAuditable} to override the column names used on database. It also uses OffsetDateTime
+ * for created and modified. The datetimes will be converted from tenant TZ to UTC before storing (automatically
+ * happens) and converted from System TZ to tenant TZ after fetching from DB
+ *
+ * Abstract base class for auditable entities. Stores the audit values in persistent fields.
+ */
+@MappedSuperclass
+public abstract class AbstractAuditableWithUTCDateTimeCustom extends AbstractPersistableCustom
+ implements Auditable<Long, Long, OffsetDateTime> {
+
+ private static final long serialVersionUID = 141481953116476081L;
+
+ @Column(name = CREATED_BY_DB_FIELD, nullable = false)
+ private Long createdBy;
+
+ @Column(name = CREATED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime createdDate;
+
+ @Column(name = LAST_MODIFIED_BY_DB_FIELD, nullable = false)
+ private Long lastModifiedBy;
+
+ @Column(name = LAST_MODIFIED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime lastModifiedDate;
+
+ @Override
+ public Optional<Long> getCreatedBy() {
+ return Optional.ofNullable(this.createdBy);
+ }
+
+ @Override
+ public void setCreatedBy(final Long createdBy) {
+ this.createdBy = createdBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getCreatedDate() {
+ if (this.createdDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.createdDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.createdDate.toInstant())));
+ }
+ }
+
+ @Override
+ public void setCreatedDate(final OffsetDateTime createdDate) {
+ this.createdDate = createdDate;
+ }
+
+ @Override
+ public Optional<Long> getLastModifiedBy() {
+ return Optional.ofNullable(this.lastModifiedBy);
+ }
+
+ @Override
+ public void setLastModifiedBy(final Long lastModifiedBy) {
+ this.lastModifiedBy = lastModifiedBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getLastModifiedDate() {
+ if (this.lastModifiedDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.lastModifiedDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.lastModifiedDate.toInstant())));
Review Comment:
UTC here too?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java:
##########
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.domain;
+
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_DATE_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_DATE_DB_FIELD;
+
+import java.time.OffsetDateTime;
+import java.util.Optional;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.springframework.data.domain.Auditable;
+import org.springframework.data.jpa.domain.AbstractAuditable;
+
+/**
+ * A custom copy of {@link AbstractAuditable} to override the column names used on database. It also uses OffsetDateTime
+ * for created and modified. The datetimes will be converted from tenant TZ to UTC before storing (automatically
+ * happens) and converted from System TZ to tenant TZ after fetching from DB
+ *
+ * Abstract base class for auditable entities. Stores the audit values in persistent fields.
+ */
+@MappedSuperclass
+public abstract class AbstractAuditableWithUTCDateTimeCustom extends AbstractPersistableCustom
+ implements Auditable<Long, Long, OffsetDateTime> {
+
+ private static final long serialVersionUID = 141481953116476081L;
+
+ @Column(name = CREATED_BY_DB_FIELD, nullable = false)
+ private Long createdBy;
+
+ @Column(name = CREATED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime createdDate;
+
+ @Column(name = LAST_MODIFIED_BY_DB_FIELD, nullable = false)
+ private Long lastModifiedBy;
+
+ @Column(name = LAST_MODIFIED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime lastModifiedDate;
+
+ @Override
+ public Optional<Long> getCreatedBy() {
+ return Optional.ofNullable(this.createdBy);
+ }
+
+ @Override
+ public void setCreatedBy(final Long createdBy) {
+ this.createdBy = createdBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getCreatedDate() {
+ if (this.createdDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.createdDate
Review Comment:
Shouldn't we use UTC here?
##########
fineract-provider/src/main/java/org/springframework/data/auditing/CustomAuditingHandler.java:
##########
@@ -0,0 +1,168 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.springframework.data.auditing;
+
+import java.time.temporal.TemporalAccessor;
+import java.util.Optional;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.fineract.infrastructure.core.auditing.CustomDateTimeProvider;
+import org.apache.fineract.infrastructure.core.domain.AbstractAuditableWithUTCDateTimeCustom;
+import org.springframework.core.log.LogMessage;
+import org.springframework.data.domain.AuditorAware;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.PersistentProperty;
+import org.springframework.data.mapping.context.MappingContext;
+import org.springframework.data.mapping.context.PersistentEntities;
+import org.springframework.util.Assert;
+
+/**
+ * Due to the package-private visibility of the Auditor, temporarely The CustomAuditingHandler must be placed in the
+ * same package. Later when we don't need to distinct the Auditable entities by interface anymore, it will be reworked.
+ */
+public class CustomAuditingHandler extends AuditingHandler {
+
+ private static final Log logger = LogFactory.getLog(CustomAuditingHandler.class);
Review Comment:
Wrong Logger? Maybe just a copy paste error?
##########
fineract-provider/src/main/resources/db/changelog/tenant-store/parts/0005_jdbc_connection_string.xml:
##########
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+ <changeSet author="fineract" id="1" context="tenant_store_db">
+ <update tableName="tenant_server_connections">
+ <column name="schema_connection_parameters" value="serverTimezone=UTC&useLegacyDatetimeCode=false&sessionVariables=time_zone='-00:00'"/>
Review Comment:
Is this applicable for every DB? MySQL/MariaDB/PostgreSQL?
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -219,9 +219,9 @@ public class Loan extends AbstractPersistableCustom {
@Column(name = "submittedon_date")
private LocalDate submittedOnDate;
- @ManyToOne(optional = true, fetch = FetchType.LAZY)
- @JoinColumn(name = "submittedon_userid", nullable = true)
- private AppUser submittedBy;
+ // @ManyToOne(optional = true, fetch = FetchType.LAZY)
Review Comment:
I assume this also calls for the same comment as for in the Client class, doesn't it?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2453: Auditable loan and client and loan transaction
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2453:
URL: https://github.com/apache/fineract/pull/2453#discussion_r929639146
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java:
##########
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.domain;
+
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_DATE_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_DATE_DB_FIELD;
+
+import java.time.OffsetDateTime;
+import java.util.Optional;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.springframework.data.domain.Auditable;
+import org.springframework.data.jpa.domain.AbstractAuditable;
+
+/**
+ * A custom copy of {@link AbstractAuditable} to override the column names used on database. It also uses OffsetDateTime
+ * for created and modified. The datetimes will be converted from tenant TZ to UTC before storing (automatically
+ * happens) and converted from System TZ to tenant TZ after fetching from DB
+ *
+ * Abstract base class for auditable entities. Stores the audit values in persistent fields.
+ */
+@MappedSuperclass
+public abstract class AbstractAuditableWithUTCDateTimeCustom extends AbstractPersistableCustom
+ implements Auditable<Long, Long, OffsetDateTime> {
+
+ private static final long serialVersionUID = 141481953116476081L;
+
+ @Column(name = CREATED_BY_DB_FIELD, nullable = false)
+ private Long createdBy;
+
+ @Column(name = CREATED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime createdDate;
+
+ @Column(name = LAST_MODIFIED_BY_DB_FIELD, nullable = false)
+ private Long lastModifiedBy;
+
+ @Column(name = LAST_MODIFIED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime lastModifiedDate;
+
+ @Override
+ public Optional<Long> getCreatedBy() {
+ return Optional.ofNullable(this.createdBy);
+ }
+
+ @Override
+ public void setCreatedBy(final Long createdBy) {
+ this.createdBy = createdBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getCreatedDate() {
+ if (this.createdDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.createdDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.createdDate.toInstant())));
+ }
+ }
+
+ @Override
+ public void setCreatedDate(final OffsetDateTime createdDate) {
+ this.createdDate = createdDate;
+ }
+
+ @Override
+ public Optional<Long> getLastModifiedBy() {
+ return Optional.ofNullable(this.lastModifiedBy);
+ }
+
+ @Override
+ public void setLastModifiedBy(final Long lastModifiedBy) {
+ this.lastModifiedBy = lastModifiedBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getLastModifiedDate() {
+ if (this.lastModifiedDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.lastModifiedDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.lastModifiedDate.toInstant())));
Review Comment:
Which by the way is not happening automatically (i mean the UTC), because the OffsetDateTime will use the system TZ when the date time got fetched from the DB...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2453: Auditable loan and client and loan transaction
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2453:
URL: https://github.com/apache/fineract/pull/2453#discussion_r929651979
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java:
##########
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.domain;
+
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_DATE_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_DATE_DB_FIELD;
+
+import java.time.OffsetDateTime;
+import java.util.Optional;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.springframework.data.domain.Auditable;
+import org.springframework.data.jpa.domain.AbstractAuditable;
+
+/**
+ * A custom copy of {@link AbstractAuditable} to override the column names used on database. It also uses OffsetDateTime
+ * for created and modified. The datetimes will be converted from tenant TZ to UTC before storing (automatically
+ * happens) and converted from System TZ to tenant TZ after fetching from DB
+ *
+ * Abstract base class for auditable entities. Stores the audit values in persistent fields.
+ */
+@MappedSuperclass
+public abstract class AbstractAuditableWithUTCDateTimeCustom extends AbstractPersistableCustom
+ implements Auditable<Long, Long, OffsetDateTime> {
+
+ private static final long serialVersionUID = 141481953116476081L;
+
+ @Column(name = CREATED_BY_DB_FIELD, nullable = false)
+ private Long createdBy;
+
+ @Column(name = CREATED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime createdDate;
+
+ @Column(name = LAST_MODIFIED_BY_DB_FIELD, nullable = false)
+ private Long lastModifiedBy;
+
+ @Column(name = LAST_MODIFIED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime lastModifiedDate;
+
+ @Override
+ public Optional<Long> getCreatedBy() {
+ return Optional.ofNullable(this.createdBy);
+ }
+
+ @Override
+ public void setCreatedBy(final Long createdBy) {
+ this.createdBy = createdBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getCreatedDate() {
+ if (this.createdDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.createdDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.createdDate.toInstant())));
+ }
+ }
+
+ @Override
+ public void setCreatedDate(final OffsetDateTime createdDate) {
+ this.createdDate = createdDate;
+ }
+
+ @Override
+ public Optional<Long> getLastModifiedBy() {
+ return Optional.ofNullable(this.lastModifiedBy);
+ }
+
+ @Override
+ public void setLastModifiedBy(final Long lastModifiedBy) {
+ this.lastModifiedBy = lastModifiedBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getLastModifiedDate() {
+ if (this.lastModifiedDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.lastModifiedDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.lastModifiedDate.toInstant())));
Review Comment:
2022-07-26 13:24:12 +5:30 (tenant TZ), the date to be stored
2022-07-26 7:54:12 got stored in DB
2022-07-26 9:54:12 +2:00 (system TZ) will be the OffsetDateTime (after fetched by JPA)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2453: Auditable loan and client and loan transaction
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2453:
URL: https://github.com/apache/fineract/pull/2453#discussion_r928925804
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java:
##########
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.domain;
+
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_DATE_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_DATE_DB_FIELD;
+
+import java.time.OffsetDateTime;
+import java.util.Optional;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.springframework.data.domain.Auditable;
+import org.springframework.data.jpa.domain.AbstractAuditable;
+
+/**
+ * A custom copy of {@link AbstractAuditable} to override the column names used on database. It also uses OffsetDateTime
+ * for created and modified. The datetimes will be converted from tenant TZ to UTC before storing (automatically
+ * happens) and converted from System TZ to tenant TZ after fetching from DB
+ *
+ * Abstract base class for auditable entities. Stores the audit values in persistent fields.
+ */
+@MappedSuperclass
+public abstract class AbstractAuditableWithUTCDateTimeCustom extends AbstractPersistableCustom
+ implements Auditable<Long, Long, OffsetDateTime> {
+
+ private static final long serialVersionUID = 141481953116476081L;
+
+ @Column(name = CREATED_BY_DB_FIELD, nullable = false)
+ private Long createdBy;
+
+ @Column(name = CREATED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime createdDate;
+
+ @Column(name = LAST_MODIFIED_BY_DB_FIELD, nullable = false)
+ private Long lastModifiedBy;
+
+ @Column(name = LAST_MODIFIED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime lastModifiedDate;
+
+ @Override
+ public Optional<Long> getCreatedBy() {
+ return Optional.ofNullable(this.createdBy);
+ }
+
+ @Override
+ public void setCreatedBy(final Long createdBy) {
+ this.createdBy = createdBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getCreatedDate() {
+ if (this.createdDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.createdDate
Review Comment:
Probably we will never compare against UTC or sending it back to UI, so i thought tenant timezone would be the most beneficial here. What do you think?
##########
fineract-provider/src/main/java/org/springframework/data/auditing/CustomAuditingHandler.java:
##########
@@ -0,0 +1,168 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.springframework.data.auditing;
+
+import java.time.temporal.TemporalAccessor;
+import java.util.Optional;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.fineract.infrastructure.core.auditing.CustomDateTimeProvider;
+import org.apache.fineract.infrastructure.core.domain.AbstractAuditableWithUTCDateTimeCustom;
+import org.springframework.core.log.LogMessage;
+import org.springframework.data.domain.AuditorAware;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.PersistentProperty;
+import org.springframework.data.mapping.context.MappingContext;
+import org.springframework.data.mapping.context.PersistentEntities;
+import org.springframework.util.Assert;
+
+/**
+ * Due to the package-private visibility of the Auditor, temporarely The CustomAuditingHandler must be placed in the
+ * same package. Later when we don't need to distinct the Auditable entities by interface anymore, it will be reworked.
+ */
+public class CustomAuditingHandler extends AuditingHandler {
+
+ private static final Log logger = LogFactory.getLog(CustomAuditingHandler.class);
Review Comment:
Yes, it was definitely copy-pasted :D
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java:
##########
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.domain;
+
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_DATE_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_DATE_DB_FIELD;
+
+import java.time.OffsetDateTime;
+import java.util.Optional;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.springframework.data.domain.Auditable;
+import org.springframework.data.jpa.domain.AbstractAuditable;
+
+/**
+ * A custom copy of {@link AbstractAuditable} to override the column names used on database. It also uses OffsetDateTime
+ * for created and modified. The datetimes will be converted from tenant TZ to UTC before storing (automatically
+ * happens) and converted from System TZ to tenant TZ after fetching from DB
+ *
+ * Abstract base class for auditable entities. Stores the audit values in persistent fields.
+ */
+@MappedSuperclass
+public abstract class AbstractAuditableWithUTCDateTimeCustom extends AbstractPersistableCustom
+ implements Auditable<Long, Long, OffsetDateTime> {
+
+ private static final long serialVersionUID = 141481953116476081L;
+
+ @Column(name = CREATED_BY_DB_FIELD, nullable = false)
+ private Long createdBy;
+
+ @Column(name = CREATED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime createdDate;
+
+ @Column(name = LAST_MODIFIED_BY_DB_FIELD, nullable = false)
+ private Long lastModifiedBy;
+
+ @Column(name = LAST_MODIFIED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime lastModifiedDate;
+
+ @Override
+ public Optional<Long> getCreatedBy() {
+ return Optional.ofNullable(this.createdBy);
+ }
+
+ @Override
+ public void setCreatedBy(final Long createdBy) {
+ this.createdBy = createdBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getCreatedDate() {
+ if (this.createdDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.createdDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.createdDate.toInstant())));
+ }
+ }
+
+ @Override
+ public void setCreatedDate(final OffsetDateTime createdDate) {
+ this.createdDate = createdDate;
+ }
+
+ @Override
+ public Optional<Long> getLastModifiedBy() {
+ return Optional.ofNullable(this.lastModifiedBy);
+ }
+
+ @Override
+ public void setLastModifiedBy(final Long lastModifiedBy) {
+ this.lastModifiedBy = lastModifiedBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getLastModifiedDate() {
+ if (this.lastModifiedDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.lastModifiedDate
+ .withOffsetSameInstant(DateUtils.getDateTimeZoneOfTenant().getRules().getOffset(this.lastModifiedDate.toInstant())));
Review Comment:
Probably we will never compare against UTC or sending it back to UI, so i thought tenant timezone would be the most beneficial here. What do you think?
##########
fineract-provider/src/main/resources/db/changelog/tenant-store/parts/0005_jdbc_connection_string.xml:
##########
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd">
+ <changeSet author="fineract" id="1" context="tenant_store_db">
+ <update tableName="tenant_server_connections">
+ <column name="schema_connection_parameters" value="serverTimezone=UTC&useLegacyDatetimeCode=false&sessionVariables=time_zone='-00:00'"/>
Review Comment:
Postgres: ignores them
Mysql: Requires the 1st
MariaDB: Requires all 3
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2453: Auditable loan and client and loan transaction
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2453:
URL: https://github.com/apache/fineract/pull/2453#discussion_r929639286
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/AbstractAuditableWithUTCDateTimeCustom.java:
##########
@@ -0,0 +1,108 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.domain;
+
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.CREATED_DATE_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_BY_DB_FIELD;
+import static org.apache.fineract.infrastructure.core.domain.AuditableFieldsConstants.LAST_MODIFIED_DATE_DB_FIELD;
+
+import java.time.OffsetDateTime;
+import java.util.Optional;
+import javax.persistence.Column;
+import javax.persistence.MappedSuperclass;
+import org.apache.fineract.infrastructure.core.service.DateUtils;
+import org.springframework.data.domain.Auditable;
+import org.springframework.data.jpa.domain.AbstractAuditable;
+
+/**
+ * A custom copy of {@link AbstractAuditable} to override the column names used on database. It also uses OffsetDateTime
+ * for created and modified. The datetimes will be converted from tenant TZ to UTC before storing (automatically
+ * happens) and converted from System TZ to tenant TZ after fetching from DB
+ *
+ * Abstract base class for auditable entities. Stores the audit values in persistent fields.
+ */
+@MappedSuperclass
+public abstract class AbstractAuditableWithUTCDateTimeCustom extends AbstractPersistableCustom
+ implements Auditable<Long, Long, OffsetDateTime> {
+
+ private static final long serialVersionUID = 141481953116476081L;
+
+ @Column(name = CREATED_BY_DB_FIELD, nullable = false)
+ private Long createdBy;
+
+ @Column(name = CREATED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime createdDate;
+
+ @Column(name = LAST_MODIFIED_BY_DB_FIELD, nullable = false)
+ private Long lastModifiedBy;
+
+ @Column(name = LAST_MODIFIED_DATE_DB_FIELD, nullable = false)
+ private OffsetDateTime lastModifiedDate;
+
+ @Override
+ public Optional<Long> getCreatedBy() {
+ return Optional.ofNullable(this.createdBy);
+ }
+
+ @Override
+ public void setCreatedBy(final Long createdBy) {
+ this.createdBy = createdBy;
+ }
+
+ @Override
+ public Optional<OffsetDateTime> getCreatedDate() {
+ if (this.createdDate == null) {
+ return Optional.empty();
+ } else {
+ return Optional.of(this.createdDate
Review Comment:
Which by the way is not happening automatically (i mean the UTC), because the OffsetDateTime will use the system TZ when the date time got fetched from the DB...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org