Modify does not modify

Reinhard

Hi developers, I am stuck on a problem, maybe you can help me?

I added a table, employment history.
When Employee status is changed, an entry is made in this table.
When an entry is made in employment history, it checks to see if
"Employment Date" "Inactive Date" or "Termination Date" needs to be updated for that employee.

By using the debugger I see that it is executing all the statements, and the employee Employment Date is correctly being set. But when I go back to card there is no change.
I have tried Modify TRUE and FALSE.
I don't want to move the code to the Employee table in case the Employment History is updated not from the employee card.

Any suggestions? Appreciate the feedback always trying to learn...

IF sEmployee.GET("Employee No.") THEN
BEGIN
  CASE Action OF

  Action::Hire:
  BEGIN
      mEmployee.GET("Employee No.");
      mEmployee."Employment Date" := Date;
      mEmployee.MODIFY(TRUE);
  END;
END;

DaveT

Hi,

I suggest that you test the return value of the MODIFY i.e.

if NOT mEmployee.MODIFY(TRUE) then
   message( 'Did not modify' );

Also I would test on the GET to avoid run-time errors.

A possibility is that the mEmployee variable is set as a Temporary table

Reinhard

I tried your suggestion, MODIFY returns TRUE.
They are not temporary tables, but they are local variables.

DaveT

Hi,

Is there code later on modifing the record, if so without another GET it would have the old Employment Date and reset it unintentionally

Reinhard

Dave, I think you are right.

onValidate on Status trigger:

EmployeeQualification.SETRANGE(...)
EmployeeQualification.MODIFYALL(...)
MODIFY;

// insert employment history:...
... 
empHistory.INSERT(TRUE);   // modifies employee record

But the MODIFY is before... but if I comment it out it works (dates are updated correctly)

OK, so then the question is how do I fix this? It would not let me COMMIT after the MODIFY in the onValidate Status trigger?

DenSter

IF sEmployee.GET("Employee No.") THEN
BEGIN
  CASE Action OF

  Action::Hire:
  BEGIN
      mEmployee.GET("Employee No.");
      mEmployee."Employment Date" := Date;
      mEmployee.MODIFY(TRUE);
      CurrForm.UPDATE; // <-- add this line
  END;
END;

The issue (I think) is that you are getting an employee record based on the current record in your form. The current record is referred to by 'Rec', which will need to be updated after you MODIFY the employee record.

By the way... why are you using two different variables for the same record?

DaveT

Hi Daniel,

Yes I think you've cracked it but the code given is on the table and the CurrForm.UPDATE; will update the form with the current value of Rec. I think the solution is to add code in the onaftervalidate trigger on the status on the form to get the record for the database again and then do the UPDATE
i.e.

GET( "No." );
currform.update( FALSE ); // Update with current Rec and don't save the old values

Reinhard

Dave, that is exactly what I did and it works great. Absolutely awsome, I would NEVER have gotten this on my own.

The only reason I have two variables is because I wanted to test the very remote chance that this was causing the problem... (so one for search, one for modification)

kine

Just a hint... why the code is on the form? Isn't it better to have it in the table or in some codeunit? For me it looks like generic function for the table, and not some specific function for the form...

Reinhard

You're right, all of the code is in the table triggers. However I had to add the two lines from Dave's post to the Form, otherwise it would overwrite the table triggers' modifcations with it's outdated version of the record.

DenSter

So make the changes straight on Rec and you wouldn't have to GET anything.

kine

DenSter wrote:

So make the changes straight on Rec and you wouldn't have to GET anything.

Yes, this is what I wanted to pinpoint too... ;-) Such problems are based on wrong concept of the code. :whistle:

Reinhard

Is it a good idea to make changes to Rec in this example?

For example, I could reverse the logic. First Insert into employment history, then do something like

(Rec.)"Employment Date" := empHistory.getEmploymentDate("No.");

which would also be a good solution. Maybe cleaner even. But then this logic would have to be repeated anytime you modify the History table, so to avoid that I figured it would make more sense the other way around, and all you have to worry about is keeping the history up to date. Also if history is edited directly, Employee records stay up to date as well. Turned out to not be so simple! Like I said at the top, always trying to learn, best case scenario my question (and your help) actually leads to a better understanding of the code for me, so it is appreciated.

kine

If the date on Employee card must be changed when history of the employee is changed, than best place for this will be in OnInsert/OnModify of the history table or some similar trigger. In this way you can synchronize the tables...

DaveT

kine wrote:

If the date on Employee card must be changed when history of the employee is changed, than best place for this will be in OnInsert/OnModify of the history table or some similar trigger. In this way you can synchronize the tables...

I agree but if you look at this example the date is changing as a result of changing the status instead of the history so putting the code in the OnInsert/OnModify of the history table will still give the same result/error. The question is - does the history get changed at all and/or from other sources? If not then the date should be changed from the Onvalidate of the Status on the table.

kine

Do not forget that if you change another table from within OnValidate, you can end with inconsystent data, becuase the OnValidate will succeed, the transaction will be commited, but if e.g. OnModify of the table will fail, the changed field on which the OnValidate was called, will have still old value. It is why I am offering to change the realted table in OnModify and OnInsert of the history table.

DenSter

I think I would opt for an entirely different design altogether. I'd look at using a flowfield, or at having a function in the Employee table that returns the proper date to display on a form, so you can set that as the SourceExpr on forms. I would never opt for a solution that updates a field in a master table with a value from a related transactional table. The thing is, you can't guarantee the accuracy of that field either way, NAV just doesn't have the right form events for that, and there's always a situation in which the records could potentially be out of synch. The way that this works is completely different than any standard NAV process, and it feels counterintuitive to me (in a NAV user sense). Try to come up with a solution that 'feels' like it is part of the standard application.

kine

Yes, that's another way, if you do not need t filter on the field. All depends on details...

DenSter

kine wrote:

All depends on details...

Ain't that the truth

Reinhard

I have enjoyed reading all your feedback it has been a good discussion, thank you