feature: Background updates on mobile #5953
Conversation
|
@Julesssss I need a little help with testing this in iOS. I am stuck where I am able to run basic console.log etc but not able to do the API call. The job always times out and then |
|
Sorry @mananjadhav, I didn't get to this task this week. I'd be happy to investigate next week though! |
|
@Julesssss Bump |
|
Thanks for the bump, I've been pretty busy this week and didn't manage to get to this. But I'll definitely be able to review next week. |
|
Sure. Quick question outside the PR, do we have a process where if contributors want to go on vacation they can notify? |
We don't at the moment, but I don't think it's a problem here as I've caused some delays here myself. |
|
Hey @mananjadhav. Once again, sorry for my long delay here. Now that I'm back and caught up I will make sure not to delay us again. Do you remember what exactly was failing? I'm seeing a lot of these errors in both iOS and Android:
|
|
I'll check and get back to you. But as far as I remember the task was getting initialized but was getting timed out. |
kidroca
left a comment
kidroca
left a comment
There was a problem hiding this comment.
Left some notes mostly on changes that shouldn't have been committed
| const headlessTask = async (event) => { // eslint-disable-line | ||
|
|
||
| // Get task id from event {}: |
There was a problem hiding this comment.
The task should not be defined here in index.js
It should be defined in libs and then registered during setup or in Expensify.js didMount phase
I see this is already done, and there's a lib about this, so the code here seems like a leftover that should be removed
| DevelopmentTeam = 368M544MTT; | ||
| DevelopmentTeam = 72TRNPHV7D; | ||
| LastSwiftMigration = 1230; | ||
| ProvisioningStyle = Manual; | ||
| ProvisioningStyle = Automatic; |
There was a problem hiding this comment.
This should probably be reverted
| CODE_SIGN_IDENTITY = "iPhone Distribution"; | ||
| CODE_SIGN_STYLE = Manual; | ||
| CODE_SIGN_IDENTITY = "Apple Development"; | ||
| CODE_SIGN_STYLE = Automatic; | ||
| CURRENT_PROJECT_VERSION = 3; | ||
| DEVELOPMENT_TEAM = 368M544MTT; | ||
| DEVELOPMENT_TEAM = 72TRNPHV7D; |
There was a problem hiding this comment.
This should definitely be reverted
| PROVISIONING_PROFILE_SPECIFIER = chat_expensify_appstore; | ||
| PROVISIONING_PROFILE_SPECIFIER = ""; |
There was a problem hiding this comment.
This should definitely be reverted
| CODE_SIGN_IDENTITY = "iPhone Distribution"; | ||
| CODE_SIGN_STYLE = Manual; | ||
| CODE_SIGN_IDENTITY = "Apple Development"; | ||
| CODE_SIGN_STYLE = Automatic; | ||
| CURRENT_PROJECT_VERSION = 3; | ||
| DEVELOPMENT_TEAM = 368M544MTT; | ||
| DEVELOPMENT_TEAM = 72TRNPHV7D; |
There was a problem hiding this comment.
This should definitely be reverted
| PROVISIONING_PROFILE_SPECIFIER = chat_expensify_appstore; | ||
| PROVISIONING_PROFILE_SPECIFIER = ""; |
There was a problem hiding this comment.
This should definitely be reverted
|
@robertjchen looks like this was merged without passing tests. Please add a note explaining why this was done and remove the |
|
Wait, what happened here? I merged #6641 but somehow this was merged as well? 🤔 |
|
I'm going to revert this- this was not supposed to be merged. |
|
Wow, that was unexpected! It's my fault, as my PR accidentally branched from this PR. I made a lot of changes and rebased a couple of times, which made removing these commits very time-consuming (I didn't notice this immediately).
|
|
Wooahh, let me close this PR and raise one more as draft that shouldn't have allow merging? |
|
Hi @mananjadhav. Don't worry, I think you can just create a new PR. You might need to cherry-pick the commits though, so they have a new commit ID. I had a large PR that accidentally branched from yours, I tried to rebase them away but was unsuccessful as it was such a large PR with many conflicts over the last month. I think the draft status was ignored by Github as technically your commits are now in |
|
I'll cherry-pick or move the commits as a fresh set of changes. Tracking my branch separately. |
|
Thanks, @mananjadhav. Sorry for the confusion. |
5 participants
cc - @Julesssss
Details
Fixed Issues
$ #4264
Tests
QA Steps
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android