Commit aa120022 authored by Cristian Berner's avatar Cristian Berner 💎 Committed by Asier Lostalé
Browse files

Fixes ISSUE-45341: OBInterceptor considers an update if audit manually set on new record

New records were considered by OBInterceptor as an update if the
createdBy audit column was set manually and it also thrown out a
NullPointerException if OBInterceptor.setPreventUpdateInfoChange was set
to true previous to the .save.

This was happening because OBInterceptor assumed(wrongly) that createdBy
field was enough to check if an object was new. This has been modified
to check also the other 3 audit fields, if null, it will be considered
new and execution will happen onNew method, as expected.

Also, onNew method has been modified to update data only if it was not
already set previously manually, that means, when those audit
creationDate, createdBy, updated and updatedBy are null.
parent f9de9389
......@@ -11,7 +11,7 @@
* under the License.
* The Original Code is Openbravo ERP.
* The Initial Developer of the Original Code is Openbravo SLU
* All portions are Copyright (C) 2015-2017 Openbravo SLU
* All portions are Copyright (C) 2015-2020 Openbravo SLU
* All Rights Reserved.
* Contributor(s): ______________________________________.
************************************************************************
......@@ -20,6 +20,8 @@
package org.openbravo.base.weld.test.testinfrastructure;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThat;
import org.jboss.arquillian.junit.InSequence;
......@@ -30,9 +32,14 @@ import org.openbravo.base.exception.OBException;
import org.openbravo.base.provider.OBProvider;
import org.openbravo.client.application.test.event.ObserverBaseTest;
import org.openbravo.client.application.test.event.OrderLineTestObserver;
import org.openbravo.dal.core.OBContext;
import org.openbravo.dal.core.OBInterceptor;
import org.openbravo.dal.service.OBDal;
import org.openbravo.erpCommon.utility.OBMessageUtils;
import org.openbravo.model.ad.access.User;
import org.openbravo.model.common.geography.Country;
import org.openbravo.test.base.Issue;
import org.openbravo.test.base.TestConstants;
/**
* Persistance observers require of cdi. Test cases covering observers are executed when using
......@@ -94,4 +101,35 @@ public class DalPersistanceEventTest extends ObserverBaseTest {
OBDal.getInstance().rollbackAndClose();
}
}
@Test
@InSequence(5)
@Issue("45341")
public void persistenceObserversShouldBeExecutedOnModifiedCreatedBy() {
try {
setSystemAdministratorContext();
Country newCountry = OBProvider.getInstance().get(Country.class);
newCountry.setName("Wonderland");
newCountry.setISOCountryCode("WL");
newCountry.setAddressPrintFormat("-");
// Set createdBy user id manually
User user = OBDal.getInstance().get(User.class, TestConstants.Users.OPENBRAVO);
newCountry.setCreatedBy(user);
OBInterceptor.setPreventUpdateInfoChange(true);
OBDal.getInstance().save(newCountry);
OBDal.getInstance().flush();
// createdBy user should be persisted and should be different than updatedBy value
String createdById = newCountry.getCreatedBy().getId();
String updatedById = newCountry.getUpdatedBy().getId();
assertEquals("createdBy value is not the one assigned manually.", user.getId(), createdById);
assertEquals("updatedBy value has not been assigned using OBContext user.",
OBContext.getOBContext().getUser().getId(), updatedById);
assertNotEquals("createdBy and updatedBy are equal, those should be different.", createdById,
updatedById);
} finally {
OBDal.getInstance().rollbackAndClose();
}
}
}
......@@ -11,7 +11,7 @@
* under the License.
* The Original Code is Openbravo ERP.
* The Initial Developer of the Original Code is Openbravo SLU
* All portions are Copyright (C) 2008-2019 Openbravo SLU
* All portions are Copyright (C) 2008-2020 Openbravo SLU
* All Rights Reserved.
* Contributor(s): ______________________________________.
************************************************************************
......@@ -333,7 +333,9 @@ public class OBInterceptor extends EmptyInterceptor {
}
final Traceable t = (Traceable) object;
if (t.getCreatedBy() == null) { // new
boolean isNew = t.getCreatedBy() == null || t.getCreationDate() == null
|| t.getUpdated() == null || t.getUpdatedBy() == null;
if (isNew) {
onNew(t, propertyNames, currentState);
} else {
onUpdate(t, propertyNames, currentState);
......@@ -380,19 +382,21 @@ public class OBInterceptor extends EmptyInterceptor {
currentState[i] = currentDate;
}
if (PROPERTY_UPDATED.equals(propertyNames[i])) {
if (PROPERTY_UPDATED.equals(propertyNames[i]) && traceable.getUpdated() == null) {
traceable.setUpdated(currentDate);
currentState[i] = currentDate;
}
if (PROPERTY_UPDATEDBY.equals(propertyNames[i])) {
if (PROPERTY_UPDATEDBY.equals(propertyNames[i]) && traceable.getUpdatedBy() == null) {
traceable.setUpdatedBy(currentUser);
currentState[i] = currentUser;
}
if (Organization.PROPERTY_CREATIONDATE.equals(propertyNames[i])) {
if (Organization.PROPERTY_CREATIONDATE.equals(propertyNames[i])
&& traceable.getCreationDate() == null) {
traceable.setCreationDate(currentDate);
currentState[i] = currentDate;
}
if (Organization.PROPERTY_CREATEDBY.equals(propertyNames[i])) {
if (Organization.PROPERTY_CREATEDBY.equals(propertyNames[i])
&& traceable.getCreatedBy() == null) {
traceable.setCreatedBy(currentUser);
currentState[i] = currentUser;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment