Skip to content

Commit

Permalink
Merge branch 'main' into feat/old-dot-submodule
Browse files Browse the repository at this point in the history
  • Loading branch information
staszekscp committed Nov 22, 2024
2 parents 56b8924 + 3611198 commit 0fdf7c4
Show file tree
Hide file tree
Showing 271 changed files with 6,515 additions and 2,563 deletions.
1 change: 1 addition & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ONYX_METRICS=false
USE_THIRD_PARTY_SCRIPTS=false

EXPENSIFY_ACCOUNT_ID_ACCOUNTING=-1
EXPENSIFY_ACCOUNT_ID_ACCOUNTS_PAYABLE=-1
EXPENSIFY_ACCOUNT_ID_ADMIN=-1
EXPENSIFY_ACCOUNT_ID_BILLS=-1
EXPENSIFY_ACCOUNT_ID_CHRONOS=-1
Expand Down
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
- [ ] I verified that comments were added to code that is not self explanatory
- [ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- [ ] I verified any copy / text shown in the product is localized by adding it to `src/languages/*` files and using the [translation method](https://github.com/Expensify/App/blob/4bd99402cebdf4d7394e0d1f260879ea238197eb/src/components/withLocalize.js#L60)
- [ ] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- [ ] If any non-english text was added/modified, I used [JaimeGPT](https://chatgpt.com/g/g-2dgOQl5VM-english-to-spanish-translator-aka-jaimegpt) to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- [ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the [localization methods](https://github.com/Expensify/App/blob/4bd99402cebdf4d7394e0d1f260879ea238197eb/src/components/withLocalize.js#L60-L68)
- [ ] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- [ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/androidBump.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
name: Android Rollout Bumper

on:
workflow_dispatch:
schedule:
# Runs at midnight every day
- cron: '0 0 * * *'
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/buildAndroid.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ jobs:
build:
name: Build Android app
runs-on: ubuntu-latest-xl
env:
RUBYOPT: '-rostruct'
outputs:
AAB_FILE_NAME: ${{ steps.build.outputs.AAB_FILE_NAME }}
APK_FILE_NAME: ${{ steps.build.outputs.APK_FILE_NAME }}
Expand Down
4 changes: 0 additions & 4 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ jobs:
name: Upload Android build to Google Play Store
needs: buildAndroid
runs-on: ubuntu-latest
env:
RUBYOPT: '-rostruct'
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down Expand Up @@ -121,8 +119,6 @@ jobs:
needs: prep
if: ${{ github.ref == 'refs/heads/production' }}
runs-on: ubuntu-latest
env:
RUBYOPT: '-rostruct'
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down
20 changes: 18 additions & 2 deletions .github/workflows/testBuild.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

postGitHubCommentBuildStarted:
runs-on: ubuntu-latest
needs: [validateActor, getBranchRef]
if: ${{ fromJSON(needs.validateActor.outputs.READY_TO_BUILD) }}
steps:
- name: Add build start comment
uses: actions/github-script@v7
with:
github-token: ${{ github.token }}
script: |
const workflowURL = `https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: process.env.PULL_REQUEST_NUMBER,
body: `🚧 @${{ github.actor }} has triggered a test build. You can view the [workflow run here](${workflowURL}).`
});
buildAndroid:
name: Build Android app for testing
uses: ./.github/workflows/buildAndroid.yml
Expand All @@ -75,8 +93,6 @@ jobs:
name: Upload Android app to S3
needs: [buildAndroid]
runs-on: ubuntu-latest
env:
RUBYOPT: '-rostruct'
outputs:
S3_APK_PATH: ${{ steps.exportS3Path.outputs.S3_APK_PATH }}
steps:
Expand Down
4 changes: 2 additions & 2 deletions android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ android {
minSdkVersion rootProject.ext.minSdkVersion
targetSdkVersion rootProject.ext.targetSdkVersion
multiDexEnabled rootProject.ext.multiDexEnabled
versionCode 1009006204
versionName "9.0.62-4"
versionCode 1009006503
versionName "9.0.65-3"
// Supported language variants must be declared here to avoid from being removed during the compilation.
// This also helps us to not include unnecessary language variants in the APK.
resConfigs "en", "es"
Expand Down
28 changes: 28 additions & 0 deletions assets/images/simple-illustrations/simple-illustration__pillow.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion contributingGuides/PERFORMANCE_METRICS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Project is using Firebase for tracking these metrics. However, not all of them a
| `js_loaded` || The time it takes for the JavaScript bundle to load. <br><br>**Platforms:** Android, iOS | **Android:** Starts in the `onCreate` method.<br><br>**iOS:** Starts in the AppDelegate's `didFinishLaunchingWithOptions` method. | Stops at the first render of the app via native module on the JS side. |
| `_app_in_foreground` || The time when the app is running in the foreground and available to the user.<br><br>**Platforms:** Android, iOS | **Android:** Starts when the first activity to reach the foreground has its `onResume()` method called. <br><br>**iOS:** Starts when the application receives the `UIApplicationDidBecomeActiveNotification` notification. | **Android:** Stops when the last activity to leave the foreground has its `onStop()` method called. <br><br>**iOS:** Stops when it receives the `UIApplicationWillResignActiveNotification` notification. |
| `_app_in_background` || Time when the app is running in the background.<br><br>**Platforms:** Android, iOS | **Android:** Starts when the last activity to leave the foreground has its `onStop()` method called. <br><br>**iOS:** Starts when the application receives the `UIApplicationWillResignActiveNotification` notification. | **Android:** Stops when the first activity to reach the foreground has its `onResume()` method called. <br><br>**iOS:** Stops when it receives the `UIApplicationDidBecomeActiveNotification` notification. |
| `sidebar_loaded` | | Time taken for the Sidebar to load.<br><br>**Platforms:** All | Starts when the Sidebar is mounted. | Stops when the LHN finishes laying out. |
| `sidebar_loaded` | | Time taken for the Sidebar to load.<br><br>**Platforms:** All | Starts when the Sidebar is mounted. | Stops when the LHN finishes laying out. |
| `calc_most_recent_last_modified_action` || Time taken to find the most recently modified report action or report.<br><br>**Platforms:** All | Starts when the app reconnects to the network | Ends when the app reconnects to the network and the most recent report action or report is found. |
| `open_search` || Time taken to open up the Search Router.<br><br>**Platforms:** All | Starts when the Search Router icon in LHN is pressed. | Stops when the list of available options finishes laying out. |
| `load_search_options` || Time taken to generate the list of options used in the Search Router.<br><br>**Platforms:** All | Starts when the `getSearchOptions` function is called. | Stops when the list of available options is generated. |
Expand Down
2 changes: 1 addition & 1 deletion contributingGuides/STORYBOOK.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export {

That will give us an interactive playground to test out various component attributes with the defaults we passed.

![Storybook example](web/storybook-example.png)
![Storybook example](/web/storybook-example.png)

Note that we did not need to write any of the descriptions for these props. This is because they are automatically generated from a React component's `propTypes`.

Expand Down
64 changes: 56 additions & 8 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,20 +477,68 @@ if (ref.current && 'getBoundingClientRect' in ref.current) {

### Default value for inexistent IDs

Use `'-1'` or `-1` when there is a possibility that the ID property of an Onyx value could be `null` or `undefined`.
Use `CONST.DEFAULT_NUMBER_ID` when there is a possibility that the number ID property of an Onyx value could be `null` or `undefined`. **Do not default string IDs to any value!**

> Why? The default number ID (currently set to `0`, which matches the backend’s default) is a falsy value. This makes it compatible with conditions that check if an ID is set, such as `if (!ownerAccountID) {`. Since it’s stored as a constant, it can easily be changed across the codebase if needed.
>
> However, defaulting string IDs to `'0'` breaks such conditions because `'0'` is a truthy value in JavaScript. Defaulting to `''` avoids this issue, but it can cause crashes or bugs if the ID is passed to Onyx. This is because `''` could accidentally subscribe to an entire Onyx collection instead of a single record.
>
> To address both problems, string IDs **should not have a default value**. This approach allows conditions like `if (!policyID) {` to work correctly, as `undefined` is a falsy value. At the same time, it prevents Onyx bugs: if `policyID` is used to subscribe to a specific Onyx record, a `policy_undefined` key will be used, and Onyx won’t return any records.
>
> In case you are confused or find a situation where you can't apply the rules mentioned above, please raise your question in the [`#expensify-open-source`](https://expensify.slack.com/archives/C01GTK53T8Q) Slack channel.
``` ts
// BAD
const foo = report?.reportID ?? '';
const bar = report?.reportID ?? '0';

report ? report.reportID : '0';
report ? report.reportID : '';
const accountID = report?.ownerAccountID ?? -1;
const policyID = report?.policyID ?? '-1';
const managerID = report ? report.managerID : 0;

// GOOD
const foo = report?.reportID ?? '-1';
const accountID = report?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID;
const policyID = report?.policyID;
const managerID = report ? report.managerID : CONST.DEFAULT_NUMBER_ID;
```
Here are some common cases you may face when fixing your code to remove the old/bad default values.
#### **Case 1**: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
```diff
-Report.getNewerActions(newestActionCurrentReport?.reportID ?? '-1', newestActionCurrentReport?.reportActionID ?? '-1');
+Report.getNewerActions(newestActionCurrentReport?.reportID, newestActionCurrentReport?.reportActionID);
```
> error TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'. Type 'undefined' is not assignable to type 'string'.
We need to change `Report.getNewerActions()` arguments to allow `undefined`. By doing that we could add a condition that return early if one of the parameters are falsy, preventing the code (which is expecting defined IDs) from executing.
```diff
-function getNewerActions(reportID: string, reportActionID: string) {
+function getNewerActions(reportID: string | undefined, reportActionID: string | undefined) {
+ if (!reportID || !reportActionID) {
+ return;
+ }
```
#### **Case 2**: Type 'undefined' cannot be used as an index type.
```diff
function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = false, updatedTransaction, isFromReviewDuplicates = false}: MoneyRequestViewProps) {
const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`, {
canEvict: false,
});
- const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1'];
+ const parentReportAction = parentReportActions?.[report?.parentReportActionID];
```
> error TS2538: Type 'undefined' cannot be used as an index type.
This error is inside a component, so we can't simply use early return conditions here. Instead, we can check if `report?.parentReportActionID` is defined, if it is we can safely use it to find the record inside `parentReportActions`. If it's not defined, we just return `undefined`.
report ? report.reportID : '-1';
```diff
function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = false, updatedTransaction, isFromReviewDuplicates = false}: MoneyRequestViewProps) {
- const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1'];
+ const parentReportAction = report?.parentReportActionID ? parentReportActions?.[report.parentReportActionID] : undefined;
```
### Extract complex types
Expand Down
Loading

0 comments on commit 0fdf7c4

Please sign in to comment.