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

Add device update status/last update status event that devices can report #1692

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

Page-
Copy link
Contributor

@Page- Page- commented Jun 28, 2024

No description provided.

@Page- Page- force-pushed the device-update-status branch 2 times, most recently from a0dff8c to 878882b Compare July 1, 2024 17:23
@Page- Page- changed the title WIP: device update status Add device update status/last update status event that devices can report Jul 1, 2024
@Page- Page- requested a review from a team July 1, 2024 17:55
@Page- Page- force-pushed the device-update-status branch 3 times, most recently from e5fb3f0 to edff86a Compare July 4, 2024 19:04
@Page- Page- force-pushed the device-update-status branch 3 times, most recently from 01fbab0 to 399c0be Compare July 11, 2024 11:25
@@ -319,6 +330,22 @@ export const statePatchV3: RequestHandler = async (req, res) => {
deviceBody.is_running__release = release.id;
}
}
const { releases } = apps[userAppUuid];
Copy link
Member

@thgreasi thgreasi Aug 9, 2024

Choose a reason for hiding this comment

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

Since userAppUuid = device.belongs_to__application[0].uuid should we first check whether apps[userAppUuid] is actually there before destructuring it?

Suggested change
const { releases } = apps[userAppUuid];
const releases = apps[userAppUuid]?.releases;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this will only happen in the case that userAppUuid is null, which should be impossible, but I'll add the check regardless

Copy link
Contributor Author

@Page- Page- Aug 13, 2024

Choose a reason for hiding this comment

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

So interestingly this change you proposed triggers an error

src/features/device-state/routes/state-patch-v3.ts:321:11 - error TS7022: 'device' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.

@thgreasi, I'm not sure why though. And as a check

const releases = apps[userAppUuid].releases;

does not have any issues

Copy link
Member

@thgreasi thgreasi left a comment

Choose a reason for hiding this comment

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

Other than that LGTM 👍

@Page- Page- force-pushed the device-update-status branch 4 times, most recently from 7aacd49 to a2ae396 Compare August 12, 2024 15:36
@Page- Page- requested a review from thgreasi August 12, 2024 15:41
@Page- Page- marked this pull request as ready for review August 12, 2024 15:41
@Page- Page- marked this pull request as draft August 12, 2024 15:41
@Page- Page- marked this pull request as ready for review August 12, 2024 15:41
@flowzone-app flowzone-app bot enabled auto-merge August 13, 2024 14:09
@Page- Page- requested a review from a team August 13, 2024 17:05
@flowzone-app flowzone-app bot merged commit c69baa6 into master Aug 19, 2024
50 checks passed
@flowzone-app flowzone-app bot deleted the device-update-status branch August 19, 2024 08:47
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.

3 participants