Fix navigating to workspace by preloading betas #5443
Conversation
|
|
src/libs/actions/User.js
Outdated
| /** | ||
| * @return {Promise} | ||
| */ | ||
| function getBetas() { | ||
| API.User_GetBetas().then((response) => { | ||
| return API.User_GetBetas().then((response) => { |
There was a problem hiding this comment.
Returning from an Action function (that is consumed OUTSIDE of the Action file) breaks our architecture.
There was a problem hiding this comment.
Oh fuck, you are right!
There was a problem hiding this comment.
It's a pain, but I see two alternatives:
- Pass a flag to
getBetas(shouldCloseModal)and then close the modals within the action - Add an Onyx key, subscribe in
LogInWithShortLivedTokenPage
|
Updated and retested |
| class LogInWithShortLivedTokenPage extends Component { | ||
| constructor(props) { | ||
| super(props); | ||
| this.state = {run: false}; |
There was a problem hiding this comment.
I don't really get run for the state variable, should it be hasRan?
There was a problem hiding this comment.
Yes, hasRun would be a better name.
There was a problem hiding this comment.
I haven't tested the latest changes yet, but this seems much more complicated than simply adding a new Action function:
function getBetasAndCloseModal() {
getBetas().then(() => {
Navigation.dismissModal();
Navigation.navigate(exitTo);
};
}
This is pretty bad, but at least it is simple.
IMO that's just a hack. Getting betas has nothing to do with modals or navigating to another page. I even prefer the previous solution to that. |
| // and by calling dismissModal(), the /transition/... route is removed from history so the user will get taken to `/` | ||
| // if they cancel out of the new workspace modal. | ||
| Navigation.dismissModal(); | ||
| Navigation.navigate(exitTo); |
There was a problem hiding this comment.
Final comment, do we need to call this.setState({run: true}) here too, to prevent other GetBeta API calls from triggering Navigation.navigate a second time?
There was a problem hiding this comment.
As the GetBetas API call is now happening twice
There was a problem hiding this comment.
As the GetBetas API call is now happening twice
Fixed that
Final comment, do we need to call this.setState({run: true}) here too, to prevent other GetBeta API calls from triggering Navigation.navigate a second time?
We can't since you should not call setState from componentDidUpdate according to linter. In any case it is not needed since once we navigate this component stops existing.
|
I'm seeing TestingPolicyRedirect.movClearing session on new.dot resolved the issue though. |
|
ok, updated one more time and retested it. The call to getBetas was not really needed as that is loaded once we are signed in (either because we already were or after we create the login). |
(cherry picked from commit 874f694)
|
🚀 Cherry-picked to staging by @iwiznia in version: 1.1.1-6 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
@iwiznia I'm getting a blank screen and a JS error after creating a Free Plan policy on this step |
|
That's odd, I just tested it all and worked. Maybe it's related to the account? Can you share the credentials to access it? |
|
So weird I just tried now and it worked 🤯 Tested all the steps and they were a pass 🎉 I'm double checking with the team |
|
Checking it off since it was a pass 🎉 |
Details
Fixed Issues
Issue uncovered while testing #5439 in stg
Tests
isDevelopment() ||from hereApp/src/libs/Permissions.js
Line 11 in 0e6fdbf
Tested On
Screenshots