Skip to content

Conversation

adalpari
Copy link
Contributor

@adalpari adalpari commented Oct 8, 2025

Description

Closes CMM-828 and CMM-829

This PR implements CRUD (Create, Read, Update, Delete) functionality for the taxonomies data view feature in WordPress Android. It allows users to create new terms, edit existing terms, and delete terms through the current Dataview.

Extra notes:

  • The architecture has been moved to use a more MVVM approach
  • Navigation has been completely moved inside the ViewModel as well.

Testing instructions

  1. Log into a self-hosted site with Application Password
  2. Open Site Settings
  • Open any of the current taxonomies and smoke test the CRUD features
Screen.Recording.2025-10-08.at.16.46.10.mov

NOTE: There's a UI glitch related to the top bar being displaced when opening the keyboard. But that glitch is also happening in trunk. So I'm better isolating it in a different branch.

@adalpari adalpari requested a review from Copilot October 8, 2025 12:57
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements CRUD (Create, Read, Update, Delete) functionality for the taxonomies data view feature in WordPress Android. It allows users to create new terms, edit existing terms, and delete terms through a comprehensive UI.

  • Added create and update term functionality with form-based editing
  • Implemented term deletion with confirmation dialogs
  • Enhanced navigation system with dedicated screens for term creation and editing

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
strings.xml Added new string resources for term creation, deletion, and error messages
TermsViewModel.kt Added CRUD operations, navigation logic, and state management for term editing
TermsDataViewActivity.kt Implemented editable UI forms, navigation handling, and delete confirmation dialogs

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dangermattic
Copy link
Collaborator

dangermattic commented Oct 8, 2025

3 Warnings
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 8, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr22268-d12bbac
Commitd12bbac
Direct Downloadjetpack-prototype-build-pr22268-d12bbac.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 8, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr22268-d12bbac
Commitd12bbac
Direct Downloadwordpress-prototype-build-pr22268-d12bbac.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Oct 8, 2025

Codecov Report

❌ Patch coverage is 29.77099% with 92 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.88%. Comparing base (783d50e) to head (d12bbac).
⚠️ Report is 2 commits behind head on trunk.

Files with missing lines Patch % Lines
.../wordpress/android/ui/taxonomies/TermsViewModel.kt 30.95% 80 Missing and 7 partials ⚠️
...ess/android/ui/taxonomies/TermsDataViewActivity.kt 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #22268      +/-   ##
==========================================
- Coverage   39.89%   39.88%   -0.01%     
==========================================
  Files        2168     2168              
  Lines      102699   102821     +122     
  Branches    14813    14830      +17     
==========================================
+ Hits        40968    41007      +39     
- Misses      58261    58337      +76     
- Partials     3470     3477       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adalpari adalpari marked this pull request as ready for review October 8, 2025 14:49
Copy link

sonarqubecloud bot commented Oct 8, 2025

Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

LGTM. The test plan succeeded for me. I did not encounter any issues.

I'll not some improvement opportunities we could consider in the future:

  • Auto-capitalize the description fields, possibly the name fields as well.
  • Add unique, descriptive a11y labels to all inputs making it clear which data field they control; currently, all are described in the same, generic "empty edit box" label.
  • Add a11y announcement any time asynchronous work occurs, particularly the success/failure of adding/editing/deleting records.
  • Consider grouping each label and input set into a single a11y item to reduce the number of "steps" in the forms.
  • When persisting a record fails, display (and a11y announce) specific validation errors that prevented success.
  • Evaluate whether we should use forward/back slides transitions rather than cross-fade transitions for navigating up/down these CRUD views.
  • This may already be recognized, but, overall, the DataView UI feels unpolished; we should consider requesting a designer provide feedback at some point.

@adalpari
Copy link
Contributor Author

adalpari commented Oct 9, 2025

I'll not some improvement opportunities we could consider in the future:

  • Auto-capitalize the description fields, possibly the name fields as well.
  • Add unique, descriptive a11y labels to all inputs making it clear which data field they control; currently, all are described in the same, generic "empty edit box" label.
  • Add a11y announcement any time asynchronous work occurs, particularly the success/failure of adding/editing/deleting records.
  • Consider grouping each label and input set into a single a11y item to reduce the number of "steps" in the forms.
  • When persisting a record fails, display (and a11y announce) specific validation errors that prevented success.
  • Evaluate whether we should use forward/back slides transitions rather than cross-fade transitions for navigating up/down these CRUD views.

Thank you for your notes! I'll open a new issue to iterate over a11y stuff

  • This may already be recognized, but, overall, the DataView UI feels unpolished; we should consider requesting a designer provide feedback at some point.

I totally agree with that!

@adalpari adalpari merged commit 9ff1f8a into trunk Oct 9, 2025
23 checks passed
@adalpari adalpari deleted the feature/CMM-828-TaxonomiesDataView-second-iteration--create,-update,-delete-support branch October 9, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants