View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0025876 | Lazarus | LCL | public | 2014-03-17 20:58 | 2015-02-20 12:02 |
Reporter | Ricardo Cervera | Assigned To | Luiz Americo | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Platform | lazarus 1.0.14 | OS | Windows | ||
Summary | 0025876: DBEdit doesn´t updates LookUp fields | ||||
Description | In the example attached if I edit the record in the Grid, when I move the cursor to another field the LookUpField is updated. But if I edit the record using DBEdits, the LookUpField is not updated until the associatte DBEdit get the focus. (it doesn't matter if the DBEdit is ReadOnly or not) It looks like that when lookup field changes it does not generates deFieldChange event and TDBEdit is not notified about change. | ||||
Tags | No tags attached. | ||||
Fixed in Revision | 44505 | ||||
LazTarget | - | ||||
Widgetset | |||||
Attached Files |
|
|
|
|
The problem is related to fpc / db components not to Lazarus Currently the lookup field is not notified when the LookupDataset is changed. To fix this, one idea is to use a datalink in TField so it can be notified |
|
So, do you think this a bug or not? I don´t know exactly what I can expect and I don´t think I have the skill to solve it Perhaps I have to open a ticket in another place. |
|
It's a missing feature. Even Delphi 7 does not implements it. I prepared a patch to implement this feature but it leads to undesired side effects. IMHO is not worth implementing this feature. Attached a patch and the program to test. Also moved the report to fpc |
|
lookup.diff (8,204 bytes)
Index: packages/fcl-db/src/base/dataset.inc =================================================================== --- packages/fcl-db/src/base/dataset.inc (revision 27215) +++ packages/fcl-db/src/base/dataset.inc (working copy) @@ -83,6 +83,7 @@ procedure TDataSet.BindFields(Binding: Boolean); var i, FieldIndex: Integer; + TheLookupDataSet: TDataset; begin { FieldNo is set to -1 for calculated/lookup fields, to 0 for unbound field @@ -98,13 +99,14 @@ FOffset := FCalcFieldsSize; Inc(FCalcFieldsSize, DataSize + 1); if FieldKind in [fkLookup] then begin - if ((FLookupDataSet = nil) or (FLookupKeyFields = '') or + if ((FLookupDataSource = nil) or (FLookupKeyFields = '') or (FLookupResultField = '') or (FKeyFields = '')) then DatabaseErrorFmt(SLookupInfoError, [DisplayName]); + TheLookupDataSet := FLookupDataSource.DataSet; FFields.CheckFieldNames(FKeyFields); - FLookupDataSet.Open; - FLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields); - FLookupDataSet.FieldByName(FLookupResultField); + TheLookupDataSet.Open; + TheLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields); + TheLookupDataSet.FieldByName(FLookupResultField); if FLookupCache then RefreshLookupList; end end else begin Index: packages/fcl-db/src/base/fields.inc =================================================================== --- packages/fcl-db/src/base/fields.inc (revision 27215) +++ packages/fcl-db/src/base/fields.inc (working copy) @@ -627,8 +627,8 @@ begin if FLookupCache then Value := LookupList.ValueOfKey(FDataSet.FieldValues[FKeyFields]) - else if Assigned(FLookupDataSet) and FDataSet.Active then - Value := FLookupDataSet.Lookup(FLookupKeyfields, FDataSet.FieldValues[FKeyFields], FLookupresultField); + else if Assigned(FLookupDataSource) and FDataSet.Active then + Value := FLookupDataSource.DataSet.Lookup(FLookupKeyfields, FDataSet.FieldValues[FKeyFields], FLookupresultField); end; function TField.GetIndex: longint; @@ -645,6 +645,28 @@ Result := FieldKind = fkLookup; end; +function TField.GetLookupDataSet: TDataSet; +begin + if FLookupDataSource <> nil then + Result := FLookupDataSource.DataSet + else + Result := nil; +end; + +procedure TField.LookupDataChange(Sender: TObject; Field: TField); +begin + if (Field <> nil) and (Field = LookupDataSet.FindField(LookupResultField)) + and (DataSet <> nil) and DataSet.Active then + DataSet.CalculateFields(DataSet.ActiveBuffer); +end; + +procedure TField.LookupStateChange(Sender: TObject); +begin + //when the linked lookup dataset is destroyied a state change event is fired + if FLookupDataSource.DataSet = nil then + FreeAndNil(FLookupDataSource); +end; + function TField.GetAsLargeInt: LargeInt; begin Raise AccessError(SLargeInt); @@ -708,43 +730,36 @@ procedure TField.RefreshLookupList; var tmpActive: Boolean; + TheLookupDataset: TDataSet; begin - if not Assigned(FLookupDataSet) or (Length(FLookupKeyfields) = 0) + if not Assigned(FLookupDataSource) or (Length(FLookupKeyfields) = 0) or (Length(FLookupresultField) = 0) or (Length(FKeyFields) = 0) then Exit; - - tmpActive := FLookupDataSet.Active; + TheLookupDataset := FLookupDataSource.DataSet; + tmpActive := TheLookupDataSet.Active; try - FLookupDataSet.Active := True; + TheLookupDataSet.Active := True; FFields.CheckFieldNames(FKeyFields); - FLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields); - FLookupDataset.FieldByName(FLookupResultField); // I presume that if it doesn't exist it throws exception, and that a field with null value is still valid + TheLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields); + TheLookupDataset.FieldByName(FLookupResultField); // I presume that if it doesn't exist it throws exception, and that a field with null value is still valid LookupList.Clear; // have to be F-less because we might be creating it here with getter! - FLookupDataSet.DisableControls; + TheLookupDataSet.DisableControls; try - FLookupDataSet.First; - while not FLookupDataSet.Eof do + TheLookupDataSet.First; + while not TheLookupDataSet.Eof do begin - FLookupList.Add(FLookupDataSet.FieldValues[FLookupKeyfields], FLookupDataSet.FieldValues[FLookupResultField]); - FLookupDataSet.Next; + FLookupList.Add(TheLookupDataSet.FieldValues[FLookupKeyfields], TheLookupDataSet.FieldValues[FLookupResultField]); + TheLookupDataSet.Next; end; finally - FLookupDataSet.EnableControls; + TheLookupDataSet.EnableControls; end; finally - FLookupDataSet.Active := tmpActive; + TheLookupDataSet.Active := tmpActive; end; end; -procedure TField.Notification(AComponent: TComponent; Operation: TOperation); - -begin - Inherited Notification(AComponent,Operation); - if (Operation = opRemove) and (AComponent = FLookupDataSet) then - FLookupDataSet := nil; -end; - procedure TField.PropertyChanged(LayoutAffected: Boolean); begin @@ -995,6 +1010,22 @@ FieldKind := ValueToLookupMap[AValue]; end; +procedure TField.SetLookupDataSet(Value: TDataSet); +begin + if Value <> nil then + begin + if FLookupDataSource = nil then + begin + FLookupDataSource := TDataSource.Create(Self); + FLookupDataSource.OnDataChange := @LookupDataChange; + FLookupDataSource.OnStateChange := @LookupStateChange; + end; + FLookupDataSource.DataSet := Value; + end + else + FreeAndNil(FLookupDataSource); +end; + procedure TField.SetReadOnly(const AValue: Boolean); begin if (FReadOnly<>AValue) then Index: packages/fcl-db/src/base/db.pas =================================================================== --- packages/fcl-db/src/base/db.pas (revision 27215) +++ packages/fcl-db/src/base/db.pas (working copy) @@ -287,7 +287,7 @@ FIsIndexField : Boolean; FKeyFields : String; FLookupCache : Boolean; - FLookupDataSet : TDataSet; + FLookupDataSource : TDataSource; FLookupKeyfields : String; FLookupresultField : String; FLookupList: TLookupList; @@ -307,6 +307,9 @@ FProviderFlags : TProviderFlags; function GetIndex : longint; function GetLookup: Boolean; + function GetLookupDataSet: TDataSet; + procedure LookupDataChange(Sender: TObject; Field: TField); + procedure LookupStateChange(Sender: TObject); procedure SetAlignment(const AValue: TAlignMent); procedure SetIndex(const AValue: Longint); function GetDisplayText: String; @@ -316,6 +319,7 @@ procedure SetDisplayWidth(const AValue: Longint); function GetDisplayWidth: integer; procedure SetLookup(const AValue: Boolean); + procedure SetLookupDataSet(Value: TDataSet); procedure SetReadOnly(const AValue: Boolean); procedure SetVisible(const AValue: Boolean); function IsDisplayStored : Boolean; @@ -352,7 +356,6 @@ function GetParentComponent: TComponent; override; procedure GetText(var AText: string; ADisplayText: Boolean); virtual; function HasParent: Boolean; override; - procedure Notification(AComponent: TComponent; Operation: TOperation); override; procedure PropertyChanged(LayoutAffected: Boolean); procedure ReadState(Reader: TReader); override; procedure SetAsBCD(const AValue: TBCD); virtual; @@ -440,7 +443,7 @@ property ImportedConstraint: string read FImportedConstraint write FImportedConstraint; property KeyFields: string read FKeyFields write FKeyFields; property LookupCache: Boolean read FLookupCache write FLookupCache; - property LookupDataSet: TDataSet read FLookupDataSet write FLookupDataSet; + property LookupDataSet: TDataSet read GetLookupDataSet write SetLookupDataSet; property LookupKeyFields: string read FLookupKeyFields write FLookupKeyFields; property LookupResultField: string read FLookupResultField write FLookupResultField; property Origin: string read FOrigin write FOrigin; |
|
|
|
IMHO root of problem is somewhere else. If user updates regular field then event deFieldChange is received by procedure TDataSet.DataEvent Then in HandleFieldChange is called CalculateFields(...) CalculateFields set State=dsCalcFields and calls also CalcLookupValue for lookup field(s). CalcLookupValue assigns to lookup field new value. BUT because State is dsCalcFields for this lookup field is not generated deFieldChange ... so DB-aware controls are not notified and does not refresh displayed data (but IMO in the "lookup field" is right value) Comparing to Delphi documentation IMO there must be set State=dsCalcFields in GetCalcFields method and not in CalculateFields method. |
|
@Lacak The change that must be intercepted is not in the dataset that has the lookup field. It's in the field (fkData type) in the dataset that the lookup field references. See the attached program: Db1 has no lookup fields Db2 has a lookup fields that references a Db1 field When field referenced by the lookup (Db1.ActualField) changes, the Db2 lookup field does not change I tested in Delphi 7. I also does not update the lookup field. Technically is possible but can open a can of worms of dataset state synchronization. There's also performance considerations since, for correctness sake, all records in the lookup field should be notified, not only the active. |
|
Seems that are two different issues. One i described with changes in other dataset and other related to changes in the same dataset. I tested in Delphi and when the key field changes in the same dataset a deFieldChange event is fired for the lookup field Strangely the field instance passed to the lookup event is from the key field See log below. IntValue is a fkData field. DetailValue is a lookup field that has as key IntValue. The address in both events are 10443928 (IntValue). Somehow the ItemValue deRecordChange event is forwarded to the lookup field TMyDataLink is a TFieldDatalink descendant IntValue updated DetailDatasetRecNo:1 DataEvent (Field: IntValue) deFieldChange - Info: 10443928 TMyDataLink.RecordChanged - aField: 10443928 / IntValue DataEvent (Field: DetailValue) deFieldChange - Info: 10443928 TMyDataLink.RecordChanged - aField: 10443928 / IntValue procedure TMyDataLink.DataEvent(Event: TDataEvent; Info: Integer); begin Form1.Memo1.Lines.Add(Format('DataEvent (Field: %s) %s - Info: %d', [FieldName,DataEventNames[Event], Info])); inherited; end; procedure TMyDataLink.RecordChanged(aField: TField); var S: String; begin if aField <> nil then S:= aField.FieldName else S:= 'nil'; Form1.Memo1.Lines.Add(Format('TMyDataLink.RecordChanged - aField: %d / %s', [Integer(aField), S])); inherited; end; |
|
After further look seems that the problem is in TFieldDataLink. It receives the deFieldChange event but does not propagate. I will patch it. Moving back to Lazarus ;-) |
|
@Luiz IMO fkLookup fields should work as you describe in 0073905 not as you mentioned in 0073904 If record or value of LookupKeyFields changes in LookupDataSet it should does not affect Field which references this LookupDataSet (as you wrote also in Delphi it does not work so). Only if KeyFields changes then lookup should be performed and value of LookupResultField should be stored to our field. |
|
@Lacak To be clear, i'm also against implementing propagating changes from the lookup result since can lead to performance and dataset state issues. Initially i understand that the reporter was requesting this feature (i could not run the test application since is not database agnostic) Hopefully i fixed in TFieldDataLink |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-03-17 20:58 | Ricardo Cervera | New Issue | |
2014-03-17 20:58 | Ricardo Cervera | File Added: Lookup Error.rar | |
2014-03-18 03:08 | Luiz Americo | Note Added: 0073801 | |
2014-03-19 21:35 | Ricardo Cervera | Note Added: 0073855 | |
2014-03-21 00:47 | Luiz Americo | Note Added: 0073871 | |
2014-03-21 00:52 | Luiz Americo | File Added: lookup.diff | |
2014-03-21 00:52 | Luiz Americo | File Added: bugLookupChange.lpr | |
2014-03-21 00:53 | Luiz Americo | Project | Lazarus CCR => FPC |
2014-03-21 11:57 | LacaK | Note Added: 0073876 | |
2014-03-23 03:00 | Luiz Americo | Note Added: 0073904 | |
2014-03-23 04:06 | Luiz Americo | Note Added: 0073905 | |
2014-03-23 04:11 | Luiz Americo | Note Edited: 0073905 | View Revisions |
2014-03-23 11:22 | Luiz Americo | Note Added: 0073909 | |
2014-03-23 11:22 | Luiz Americo | Project | FPC => Lazarus |
2014-03-24 07:54 | LacaK | Note Added: 0073925 | |
2014-03-24 13:39 | Luiz Americo | Note Added: 0073931 | |
2014-03-24 13:40 | Luiz Americo | Fixed in Revision | => 44505 |
2014-03-24 13:40 | Luiz Americo | LazTarget | => - |
2014-03-24 13:40 | Luiz Americo | Widgetset | Win32/Win64 => |
2014-03-24 13:40 | Luiz Americo | Status | new => resolved |
2014-03-24 13:40 | Luiz Americo | Resolution | open => fixed |
2014-03-24 13:40 | Luiz Americo | Assigned To | => Luiz Americo |