Enable metadata editing and server sync for online objects #226
Conversation
- Made metadata fields editable in MetadataView.axaml (Description, CreatedDate, ModifiedDate) - Added two-way binding support in LocoObjectMetadataViewModel with ReactiveUI - Implemented server-side UpdateAsync endpoint in ObjectRouteHandler.cs - Added client-side UpdateObjectAsync method in Client.cs and ObjectServiceClient.cs - Integrated metadata upload in ObjectEditorViewModel.Save() for online mode Co-authored-by: LeftofZen <[email protected]>
- Updated ObjectRoutesTest to test the PUT endpoint for metadata updates - Test verifies that Description, CreatedDate, ModifiedDate, and Availability fields can be updated - Test passes successfully Co-authored-by: LeftofZen <[email protected]>
- Eliminated redundant database query in UpdateAsync endpoint - Added null check for CurrentObject.DatInfo before accessing - Improved error logging format - Removed unnecessary Task.Run wrapper Co-authored-by: LeftofZen <[email protected]>
|
@copilot this is a good start but only a few of the metadata properties are exposed in ObjectMetadataViewModel. All of them need to be exposed, editable and saveable to the server |
- Added Availability property to ObjectMetadataViewModel with two-way binding - Added InternalName property (readonly) to ViewModel - Added UploadedDate property to ViewModel - Updated MetadataView.axaml to show all properties with appropriate controls - Changed Availability from readonly TextBox to editable ComboBox - Added InternalName field display - Fixed DataContext binding to use ViewModel properties instead of direct Metadata binding - Updated type references from LocoObjectMetadataViewModel to ObjectMetadataViewModel Co-authored-by: LeftofZen <[email protected]>
All metadata properties from Editable properties (saved to server):
Readonly properties (displayed but not editable):
Collection properties (displayed in separate tabs):
All editable properties are uploaded to the server when saving in online mode. Fixed in commit ae4edac. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
- Changed UploadedDate to direct property binding (no setter) - Removed backing field for UploadedDate - Changed binding mode to OneWay in MetadataView.axaml Co-authored-by: LeftofZen <[email protected]>
|
@copilot this is great; just a few finishing touches now:
|
1. Made Availability and InternalName readonly (removed editable ComboBox for Availability) 2. Made Licence editable as ComboBox with values from /licences endpoint, includes "None" option 3. Added UI for editing Authors, Tags, and ObjectPacks with add/remove functionality 4. Added ReactiveCommands for add/remove operations 5. Fetch available Authors, Tags, ObjectPacks, and Licences from server 6. Sync collections back to Metadata model when changed Co-authored-by: LeftofZen <[email protected]>
All three finishing touches implemented in commit 22d06e0:
All collections sync changes back to the Metadata model and are uploaded to the server when saving in online mode. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR enables full metadata editing in the ObjectEditor UI and implements server synchronization for online objects. The changes include renaming LocoObjectMetadata to ObjectMetadata, implementing a comprehensive ObjectMetadataViewModel with collection editing capabilities, and adding an API endpoint for metadata updates.
Key Changes
- Introduced a new
ObjectMetadataViewModelthat exposes all editable metadata fields (Description, Licence, CreatedDate, ModifiedDate, Authors, Tags, ObjectPacks) with two-way binding and ReactiveUI change notifications - Implemented server-side
UpdateAsyncPUT endpoint and corresponding client methods to persist metadata changes when in online mode - Renamed
LocoObjectMetadatatoObjectMetadatathroughout the codebase for better naming clarity
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Gui/Views/MetadataView.axaml | Updated UI to expose all editable metadata fields with two-way bindings and added interactive collection editing (Authors, Tags, ObjectPacks) with ComboBoxes and Add/Remove buttons |
| Gui/ViewModels/ObjectMetadataViewModel.cs | New comprehensive viewmodel that wraps ObjectMetadata, provides ReactiveUI property notifications, manages server data loading, and implements collection editing commands |
| Gui/ViewModels/LocoTypes/ObjectEditorViewModel.cs | Modified Save() to call UploadMetadataAsync() for online objects, which constructs and sends DtoObjectDescriptor to the server |
| Gui/ViewModels/LocoObjectMetadataViewModel.cs | Removed the simple wrapper class (now replaced by ObjectMetadataViewModel) |
| Gui/ViewModels/FolderTreeViewModel.cs | Updated type reference from LocoObjectMetadata to ObjectMetadata |
| Gui/ObjectServiceClient.cs | Added client methods for UpdateObjectAsync, GetLicencesAsync, GetAuthorsAsync, GetTagsAsync, and GetObjectPacksAsync |
| Gui/Models/ObjectEditorModel.cs | Updated type references from LocoObjectMetadata to ObjectMetadata |
| Gui/Models/LocoUIObjectModel.cs | Updated Metadata property type from LocoObjectMetadata to ObjectMetadata |
| Gui/Converters/EnumToMaterialIconConverter.cs | Updated mapping key from LocoObjectMetadata to ObjectMetadata |
| Gui/App.axaml | Updated DataTemplate reference from LocoObjectMetadataViewModel to ObjectMetadataViewModel |
| ObjectService/RouteHandlers/TableHandlers/ObjectRouteHandler.cs | Implemented UpdateAsync endpoint to handle metadata updates and added fully-qualified type name for SourceData.ObjectMetadata |
| Definitions/Web/Client.cs | Added static helper methods for UpdateObjectAsync, GetLicencesAsync, GetAuthorsAsync, GetTagsAsync, and GetObjectPacksAsync |
| Definitions/ObjectModels/ObjectMetadata.cs | Renamed class from LocoObjectMetadata to ObjectMetadata |
| Tests/ObjectServiceIntegrationTests/ObjectRoutesTest.cs | Added PutAsync test for metadata update and improved error message for PutRequestDto |
| catch (Exception) | ||
| { | ||
| // If we can't load data (e.g., offline mode), just set empty lists | ||
| AvailableLicences = new ObservableCollection<DtoLicenceEntry?> { null }; | ||
| AvailableAuthors = new ObservableCollection<DtoAuthorEntry>(); | ||
| AvailableTags = new ObservableCollection<DtoTagEntry>(); | ||
| AvailableObjectPacks = new ObservableCollection<DtoItemPackEntry>(); | ||
| } |
There was a problem hiding this comment.
The catch block silently swallows all exceptions without logging. If the server data fails to load, users won't know why the dropdowns are empty. Consider logging the exception or providing user feedback when the data fails to load, especially since this affects critical user functionality like selecting licences, authors, tags, and object packs.
| // Load data from server if we have a client | ||
| if (objectServiceClient != null) | ||
| { | ||
| _ = LoadServerDataAsync(); |
There was a problem hiding this comment.
The LoadServerDataAsync() method is called with a fire-and-forget pattern (discarding the Task with '_' on line 82). This means any exceptions thrown after the catch block in LoadServerDataAsync won't be observed. While the current implementation catches exceptions within LoadServerDataAsync, this pattern is generally risky. Consider awaiting the task or handling unobserved exceptions appropriately.
| _ = LoadServerDataAsync(); | |
| _ = LoadServerDataAsync().ContinueWith(t => | |
| { | |
| // Observe the exception to avoid unobserved task exceptions in fire-and-forget usage. | |
| _ = t.Exception; | |
| }, TaskContinuationOptions.OnlyOnFaulted); |
| // Upload metadata to server when in online mode | ||
| if (CurrentFile.FileLocation == FileLocation.Online && CurrentFile.Id.HasValue && MetadataViewModel != null) | ||
| { | ||
| _ = UploadMetadataAsync(CurrentFile.Id.Value); |
There was a problem hiding this comment.
The UploadMetadataAsync() method is called with a fire-and-forget pattern (discarding the Task with '_' on line 358). This means the Save() method completes immediately without waiting for the upload to finish, and any errors that occur after the try-catch in UploadMetadataAsync won't block the save operation. Users won't know if the metadata upload succeeded or failed until they check logs. Consider awaiting the upload or providing UI feedback about the upload status.
| [Test] | ||
| public override async Task PutAsync() | ||
| { | ||
| // arrange | ||
| const int id = 2; | ||
| var existingObj = DbSeedData.ToList()[id - 1]; | ||
| var updatedDescription = "Updated description"; | ||
| var updatedCreatedDate = DateOnly.FromDateTime(new DateTime(2020, 1, 1)); | ||
| var updatedModifiedDate = DateOnly.FromDateTime(new DateTime(2024, 12, 15)); | ||
|
|
||
| var updateRequest = new DtoObjectDescriptor( | ||
| Id: id, | ||
| Name: existingObj.Name, | ||
| DisplayName: "test-display-name-2", | ||
| DatChecksum: null, | ||
| Description: updatedDescription, | ||
| ObjectSource: existingObj.ObjectSource, | ||
| ObjectType: existingObj.ObjectType, | ||
| VehicleType: existingObj.VehicleType, | ||
| Availability: ObjectAvailability.Available, | ||
| CreatedDate: updatedCreatedDate, | ||
| ModifiedDate: updatedModifiedDate, | ||
| UploadedDate: DateOnly.UtcToday, | ||
| Licence: null, | ||
| Authors: [], | ||
| Tags: [], | ||
| ObjectPacks: [], | ||
| DatObjects: [], | ||
| StringTable: new DtoStringTableDescriptor([], id) | ||
| ); | ||
|
|
||
| // act | ||
| var result = await ClientHelpers.PutAsync<DtoObjectDescriptor, DtoObjectDescriptor>( | ||
| HttpClient!, RoutesV2.Prefix, BaseRoute, id, updateRequest); | ||
|
|
||
| // assert | ||
| using (Assert.EnterMultipleScope()) | ||
| { | ||
| Assert.That(result, Is.Not.Null); | ||
| Assert.That(result!.Id, Is.EqualTo(id)); | ||
| Assert.That(result.Description, Is.EqualTo(updatedDescription)); | ||
| Assert.That(result.CreatedDate, Is.EqualTo(updatedCreatedDate)); | ||
| Assert.That(result.ModifiedDate, Is.EqualTo(updatedModifiedDate)); | ||
| Assert.That(result.Availability, Is.EqualTo(ObjectAvailability.Available)); | ||
| } | ||
| } |
There was a problem hiding this comment.
The PutAsync test only validates that Description, CreatedDate, ModifiedDate, and Availability are updated correctly. It doesn't test the update of Licence, Authors, Tags, or ObjectPacks collections, which are editable fields according to the PR description. Consider adding test cases that verify these fields can be updated via the API.
There was a problem hiding this comment.
Added comprehensive tests in commit 218d830:
PutAsync_UpdatesLicence()- validates Licence can be updatedPutAsync_UpdatesAuthors()- validates Authors collection can be updatedPutAsync_UpdatesTags()- validates Tags collection can be updatedPutAsync_UpdatesObjectPacks()- validates ObjectPacks collection can be updated
All 5 PutAsync tests now pass, providing full coverage of metadata update functionality.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
1. Add logging to LoadServerDataAsync exception handler - users now know why data fails to load 2. Handle fire-and-forget Tasks properly with ContinueWith to observe exceptions 3. Add comprehensive tests for PutAsync covering Licence, Authors, Tags, and ObjectPacks updates 4. Pass logger to ObjectMetadataViewModel for better error visibility All 5 PutAsync tests now pass, validating complete metadata update functionality. Co-authored-by: LeftofZen <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Metadata Editing Complete ✅
All metadata fields are now editable in the UI and can be saved to the server when in online mode.
Recent Updates (PR Review Feedback)
Editable Properties (Saved to Server)
Readonly Properties
Server API
UpdateAsyncPUT endpoint inObjectRouteHandlerClient Integration
ObjectEditorViewModel.Save()uploads all editable metadata for online objectsCollection Editing Features
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.