Repository navigation
Remove the describeRule beta - #104861
Remove the describeRule beta#104861
Conversation
Co-authored-by: Rushat Gabhane <rushatgabhane@users.noreply.github.com>
|
🤖 Web test steps verification results: On Web, the test account has no
|
Co-authored-by: Rushat Gabhane <rushatgabhane@users.noreply.github.com>
|
No code fix is needed. Every code check on the latest commit ( The only failing check is |
| }, | ||
| ]), | ||
| { | ||
| key: 'createAgentRule', |
There was a problem hiding this comment.
Can we drop this option? New rule is meant to be one flow with no up-front choice between an agent rule and a standard rule. The describe box below already drafts an agent rule when it can’t build a standard one.
Admins who know they want an agent rule still have Add AI rule / Add agent rule on the Agents tab, along with the Suggested list. It also renders under “Start with a template”, where it reads as a fifth template. Before the beta removal, this file only showed it to accounts without the beta, as a stand-in for the describe box.
There was a problem hiding this comment.
Done in 17c98ba. New rule now shows only the four rule types and then the describe section.
There was a problem hiding this comment.
@yuwenmemon One thing to flag: dropping this row removes the only direct entry to the agent rule page from New rule. The remaining paths are both on the Agents tab - the empty state CTA, or the Add agent rule row once rules exist.
That also buries the Suggested list, since it's the default tab on that page. An admin who wanted to browse suggestions now has to know to go to Rules > Agents first. The describe box can't get them there either, it only lands on Edit with a prefilled draft after Concierge decides it isn't a standard rule.
Not blocking, and the Suggested tab is still one tap away once they're on the page. Just want to confirm the extra hop is intended rather than a side effect.
| flagForReview: 'Zur Überprüfung markieren', | ||
| flagForReviewDescription: 'Benachrichtigen, wenn Ihre Bedingungen erfüllt sind', | ||
| requireFields: 'Feldanforderungen festlegen', | ||
| requireFieldsShort: 'Felder vorschreiben', |
There was a problem hiding this comment.
NAB now that requireFields is gone, we can rename the requireFieldsShort short to just be requireFields.
There was a problem hiding this comment.
Done in 8c3494b. newRule.requireFieldsShort is now newRule.requireFields in every locale.
|
@MelvinBot please get it done |
…tion" This reverts commit 5b01c88.
…title is gone Co-authored-by: Rushat Gabhane <rushatgabhane@users.noreply.github.com>
|
I addressed both of
Typecheck and lint pass, and The PR is still a draft. Before you mark it ready for review, fill in the Tests and QA sections and add screenshots. |
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 4dbbdfa7..ff399ec5 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -8897,7 +8897,7 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
applyExpenseDefaultsDescription: 'Felder automatisch aktualisieren',
flagForReview: 'Zur Überprüfung markieren',
flagForReviewDescription: 'Benachrichtigen, wenn Ihre Bedingungen erfüllt sind',
- requireFields: 'Felder vorschreiben',
+ requireFields: 'Felder erforderlich',
requireFieldsDescription: 'Belege, Kategorien usw.',
describeItBelow: 'Oder beschreiben Sie sie einfach unten',
describeRulePlaceholder: 'Alle Ausgaben über 2.000 $ blockieren',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 7bbc9a7a..ce48afe9 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -8905,7 +8905,7 @@ ${reportName}`,
applyExpenseDefaultsDescription: 'Actualizar campos automáticamente',
flagForReview: 'Marcar para revisión',
flagForReviewDescription: 'Notificar cuando se cumplan tus condiciones',
- requireFields: 'Exigir campos',
+ requireFields: 'Campos obligatorios',
requireFieldsDescription: 'Recibos, categorías, etc.',
describeItBelow: 'O simplemente descríbela abajo',
describeRulePlaceholder: 'Bloquear todos los gastos superiores a $2,000',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index e96fc2ac..aa08f7e1 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -8920,7 +8920,7 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
applyExpenseDefaultsDescription: 'Mettre à jour les champs automatiquement',
flagForReview: 'Marquer pour examen',
flagForReviewDescription: 'Avertir lorsque vos conditions sont remplies',
- requireFields: 'Exiger des champs',
+ requireFields: 'Champs obligatoires',
requireFieldsDescription: 'Reçus, catégories, etc.',
describeItBelow: 'Ou décrivez-la simplement ci-dessous',
describeRulePlaceholder: 'Bloquer toutes les dépenses supérieures à 2 000 $',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 20ca21d5..538e904e 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -8851,7 +8851,7 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`,
applyExpenseDefaultsDescription: 'Aggiorna i campi automaticamente',
flagForReview: 'Contrassegna per revisione',
flagForReviewDescription: 'Avvisa quando le tue condizioni sono soddisfatte',
- requireFields: 'Richiedi campi',
+ requireFields: 'Rendi obbligatori i campi',
requireFieldsDescription: 'Ricevute, categorie, ecc.',
describeItBelow: 'Oppure descrivila qui sotto',
describeRulePlaceholder: 'Blocca tutte le spese superiori a 2.000 $',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 5eb27fc9..40a64964 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -8736,7 +8736,7 @@ ${reportName}`,
applyExpenseDefaultsDescription: 'フィールドを自動で更新する',
flagForReview: '確認のためにフラグを付ける',
flagForReviewDescription: '条件が満たされたときに通知する',
- requireFields: '必須フィールド',
+ requireFields: '必須項目',
requireFieldsDescription: '領収書、カテゴリなど',
describeItBelow: 'または、下で説明してください',
describeRulePlaceholder: '2,000ドルを超えるすべての支出をブロック',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 1010e0f1..4828f5a7 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -8825,7 +8825,7 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`,
applyExpenseDefaultsDescription: 'Velden automatisch bijwerken',
flagForReview: 'Markeren voor controle',
flagForReviewDescription: 'Melding wanneer aan je voorwaarden is voldaan',
- requireFields: 'Velden verplichten',
+ requireFields: 'Velden verplicht stellen',
requireFieldsDescription: 'Bonnen, categorieën, enz.',
describeItBelow: 'Of beschrijf hem hieronder',
describeRulePlaceholder: 'Blokkeer alle uitgaven boven $2.000',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index f07ace52..e159ff44 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -8829,7 +8829,7 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`,
applyExpenseDefaultsDescription: 'Atualizar campos automaticamente',
flagForReview: 'Marcar para revisão',
flagForReviewDescription: 'Notificar quando suas condições forem atendidas',
- requireFields: 'Exigir campos',
+ requireFields: 'Tornar campos obrigatórios',
requireFieldsDescription: 'Recibos, categorias etc.',
describeItBelow: 'Ou simplesmente descreva abaixo',
describeRulePlaceholder: 'Bloquear todos os gastos acima de $2.000',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
@MelvinBot mark ready for review and add screenshots |
|
No code fix is needed. Every code check on the latest commit ( Two review checks still fail, and code changes can't fix either one:
|
|
@Krishna2323 @situchan One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Reviewing... |
@MelvinBot make the changes, I'll post reviewer checklist shortly |
|
I still can't make these edits.
Your reviewer checklist comment will fix the failing |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios_hybrid.mp4iOS: mWeb SafariMacOS: Chrome / Safariweb_chrome.mp4 |
|
@MelvinBot make the edits please Tests
Offline testsN/A. Describing a rule needs Concierge, so offline behavior is unchanged. QA StepsSame as tests. |
|
I updated the PR description with your steps:
The TODO placeholders are gone. Nothing else in the description changed. The PR Author Checklist boxes are still unticked, so please tick the ones you've done. |
|
@JS00001 are we ready to merge this one? |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Remove the describeRule beta (cherry picked from commit e712889) (cherry-picked to staging by yuwenmemon)
|
🚧 yuwenmemon has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/yuwenmemon in version: 9.5.20-5 🚀
|
|
Yes, the help site needs an update. This PR removes the Three articles had sentences that this PR made wrong:
I checked every label against @rushatgabhane, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.5.20-5 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Cherry-picked to staging by https://github.com/yuwenmemon in version: 9.5.21-0 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/yuwenmemon in version: 9.5.23-0 🚀
|
















Explanation of Change
Removes the
describeRulebeta so every workspace admin can describe a rule in plain English and have Concierge build it. The feature and its regression fixes are already in production, and the team agreed on the issue that the beta can go.RulesNewPage: always shows the describe section when New rule opens from the workspace Rules page and always uses the short "Require fields" title. The "Describe agent rule" option is removed, because the describe section drafts an agent rule when Concierge can't build a standard rule, and the Agents tab still has Add agent rule. Before this PR, only accounts without the beta saw that option.DescribeRuleButton: always renders the Describe button in the rule editors.DESCRIBE_RULEbeta constant, the "Describe agent rule" option and its translations, and the old longnewRule.requireFieldstranslation in every locale. RenamesnewRule.requireFieldsShorttonewRule.requireFields.RulesNewPageDescribeRuleTest: drops the beta mock and expects the four rule types followed by the describe section.Fixed Issues
$ #92743
PROPOSAL:
Tests
Offline tests
N/A. Describing a rule needs Concierge, so offline behavior is unchanged.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
New rule: four templates and the describe section

New rule: error after clicking Next with nothing selected

Require fields editor: Describe modal

Flag for review editor: Describe button

Category New rule: no describe section

AI Tests
npm run typecheck: passednpm run lint-changed: passednpm test -- tests/ui/RulesNewPageDescribeRuleTest.tsx: 16/16 passed