Skip to content
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

[Pending] feat(app) App feat add get robotName #10871

Closed
wants to merge 6 commits into from

Conversation

koji
Copy link
Contributor

@koji koji commented Jun 22, 2022

Overview

This PR is for the robot rename function and this PR will be used by #10723 to retrieve the new robot name.
This PR uses the API /server/name GET method

Changelog

  • add getRobotName to api-client
  • add useRobotName to react-api-client

Review requests

  • This PR is similar to runs and protocols with GET so the key is that the changes follow the existing coding style and usage of react-query

  • Would like to get reviewers' thought on interface that I commented

This function only works with an OT-2 (not work with robot-server)
I checked useRobotName() worked in RenameSlideout component.

Risk assessment

  • low

koji added 4 commits June 10, 2022 02:41
This PR is to add a function to get a robotName from API instead of using rename function's return.
This PR is for robot rename function and this PR will be used by #10723 to retrieve the new robot
name

partially fix #10709
@codecov
Copy link

codecov bot commented Jun 22, 2022

Codecov Report

Merging #10871 (fb3f2fb) into release_6.0.0 (1a245a8) will decrease coverage by 0.03%.
The diff coverage is 19.04%.

Impacted file tree graph

@@                Coverage Diff                @@
##           release_6.0.0   #10871      +/-   ##
=================================================
- Coverage          73.83%   73.80%   -0.04%     
=================================================
  Files               2158     2161       +3     
  Lines              58349    58659     +310     
  Branches            5963     6067     +104     
=================================================
+ Hits               43084    43291     +207     
- Misses             13991    14052      +61     
- Partials            1274     1316      +42     
Flag Coverage Δ
app 71.44% <0.00%> (-0.15%) ⬇️
react-api-client 83.33% <100.00%> (+0.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...edTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx 68.00% <ø> (ø)
...c/organisms/Devices/hooks/useRobotAnalyticsData.ts 0.00% <0.00%> (ø)
react-api-client/src/runs/useAllRunsQuery.ts 100.00% <ø> (ø)
react-api-client/src/server/useRobotName.ts 100.00% <100.00%> (ø)
...p-shell/src/protocol-analysis/executeAnalyzeCli.ts 11.11% <0.00%> (-8.89%) ⬇️
...obotSettings/UpdateBuildroot/ReleaseNotesModal.tsx 92.30% <0.00%> (-7.70%) ⬇️
app/src/organisms/Devices/RobotOverview.tsx 62.06% <0.00%> (-6.36%) ⬇️
...s/RobotSettings/AdvancedTab/RobotServerVersion.tsx 76.00% <0.00%> (-4.00%) ⬇️
app-shell/src/protocol-analysis/getPythonPath.ts 6.89% <0.00%> (-3.11%) ⬇️
app/src/organisms/Devices/RecentProtocolRuns.tsx 78.37% <0.00%> (-2.88%) ⬇️
... and 13 more

@koji koji marked this pull request as ready for review June 23, 2022 00:03
@koji koji requested review from a team as code owners June 23, 2022 00:03
@koji koji removed request for a team June 23, 2022 00:03
@koji koji changed the title feat(app) App feat add get robotName feat(app) App feat add get robotName for robotRename Jun 23, 2022
@koji koji changed the title feat(app) App feat add get robotName for robotRename feat(app) App feat add get robotName Jun 23, 2022
Comment on lines +1 to 7
export interface CurrentRobotName {
name: string
}

export interface UpdatedRobotName {
name: string
}
Copy link
Contributor Author

@koji koji Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Question to reviewers]
Should I combine these two interfaces into one like RobotName?
RobotName might be vague? In addition, we use robotName in many components so maybe a different name would be better?

@koji koji changed the title feat(app) App feat add get robotName [Pending] feat(app) App feat add get robotName Jul 5, 2022
@koji koji marked this pull request as draft August 23, 2022 05:30
@koji koji closed this Aug 24, 2022
@koji koji deleted the app_feat-add-get-robotname branch November 6, 2022 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant