-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RSDK-10127] Add get and update metadata functions #360
Conversation
lib/src/app/app.dart
Outdated
/// Retrieves user-defined [Metadata] for an organization. | ||
/// | ||
/// For more information, see [Fleet Management API](https://docs.viam.com/appendix/apis/fleet/). | ||
Future<GetOrganizationMetadataResponse> getOrganizationMetadata(String id) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(minor/nit) can we rename to organizationId
or orgId
? it's more specific and more consistent with similar calls elsewhere in the SDK.
Same comment applies to below methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great catch, thanks for calling this out
lib/src/app/app.dart
Outdated
Future<void> updateOrganizationMetadata(String id, Map<String, dynamic> data) async { | ||
final request = UpdateOrganizationMetadataRequest() | ||
..organizationId = id | ||
..data = _mapToStruct(data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_mapToStruct
is undefined, which is causing errors. I think what you're looking for is data.toStruct()
.
test('getOrganizationMetadata', () async { | ||
final expected = GetOrganizationMetadataResponse()..data = (Struct()..fields['key'] = (Value()..stringValue = 'value')); | ||
when(serviceClient.getOrganizationMetadata(any)).thenAnswer((_) => MockResponseFuture.value(expected)); | ||
final response = await appClient.getOrganizationMetadata('orgId'); | ||
expect(response, equals(expected)); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test looks good, but we should probably add tests for all the other methods you implemented as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Naveed and I were just debugging before I put all of the tests up - thanks for calling this out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Description
Add user-defined metadata CRUD operations
See ticket here