armistace/resbuilder_app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feature/my-resumes-teal #1

Closed
armistace wants to merge 0 commits from feature/my-resumes-teal into master
pull from: feature/my-resumes-teal
merge into: armistace:master
Conversation 2 Commits - Files Changed - +1426 -321
armistace commented 2026-08-04 22:45:15 +10:00
Owner
Copy Link
No description provided.
armistace added 2 commits 2026-08-04 22:45:16 +10:00 
- colors.xml: replaced legacy palette with M3 semantic tokens
  (primary, secondary, tertiary, surface containers, outline, status)
- themes.xml + values-night/themes.xml: switched to
  Theme.Material3.DayNight.NoActionBar, mapped all M3 color roles,
  transparent system bars for edge-to-edge, M3 typography
- attrs.xml: custom semantic attrs (colorSuccess, colorWarning, colorInfo)
- strings.xml: added accessibility labels, loading/empty/error strings
- activity_login.xml: centered branding, M3 headline/body, skeleton card
- activity_main.xml: edge-to-edge CoordinatorLayout, M3 AppBar/toolbar,
  health indicator group, M3 banner, M3 TabLayout
- fragment_job_form.xml: grouped fields in outlined cards (12dp radius),
  FilledBox text inputs, TonalButtons, 16dp margins
- fragment_job_status.xml: chip-style badge, LinearProgressIndicator,
  centered card with error/try-again states
- fragment_result.xml: SuggestionChip badge, WebView fills space,
  bottom action bar in outlined card
- activity_admin.xml + item_admin_user.xml: M3 card list items,
  proper type scale hierarchy
- Updated drawables to use theme attributes

Refs DESIGN.md Material Design 3 + Amazon/Google guidelines.
armistace commented 2026-08-04 22:45:18 +10:00
Author
Owner
Copy Link

PR received — starting review, sit tight 🫡

PR received — starting review, sit tight :saluting_face:
armistace commented 2026-08-04 22:47:52 +10:00
Author
Owner
Copy Link

PR Review Results

Consolidated Review Report: feature/my-resumes-teal

📋 Executive Summary

This pull request introduces the "My Resumes" feature, a comprehensive UI overhaul adopting a "teal" theme, and the integration of Google Authentication. While the functional additions are valuable, the PR currently fails basic version control and security hygiene.

The most critical issue is the inclusion of internal IDE configuration files (.idea/), which creates technical debt and security risks. Additionally, the introduction of new API endpoints for user-specific data requires verification of backend authorization (BOLA) to prevent data leaks. The lack of unit tests for new business logic is also a significant gap in quality assurance.

Overall Recommendation: 🔴 SIGNIFICANT CHANGES NEEDED

🚩 Prioritized Issue List

🔴 Critical (Immediate Action Required)

  • Version Control Pollution: Inclusion of .idea/ folder files. These must be removed and added to .gitignore to prevent configuration drift and potential leak of local system metadata.
  • Potential Secret Leakage: GOOGLE_AUTH_SETUP.md and ApiClient.kt must be audited to ensure no live Client IDs, Secrets, or API keys are committed to source control.

🟠 High Priority

  • Broken Object Level Authorization (BOLA): The new "My Resumes" endpoint must be verified on the backend to ensure users cannot access other users' resumes by manipulating IDs in the request.
  • Missing Test Coverage: No unit tests were provided for the new ApiClient logic or the MyResumesFragment business logic.
  • Architectural Concerns: Potential violation of the Single Responsibility Principle if API logic is implemented directly within Fragments/Activities instead of a ViewModel.

🟡 Medium Priority

  • Resource Management: Use of hardcoded colors instead of Theme Attributes (?attr/colorPrimary), which will hinder future Dark Mode implementation.
  • Network Security: Need to verify the presence of network_security_config.xml to enforce HTTPS and disable cleartext traffic.
  • UI Performance: Potential "over-nesting" of layouts in fragment_job_form.xml and activity_login.xml.

🔵 Low Priority

  • String Externalization: Ensure all new strings are in strings.xml for localization.
  • Model Immutability: Verify that new data models in Models.kt use val for thread safety.

🛠 Domain-Specific Recommendations

💻 Code Quality & Architecture

  • Decouple UI and Logic: Move all ApiClient calls and data transformations from LoginActivity and MyResumesFragment into a ViewModel. Use LiveData or StateFlow for UI updates.
  • Dependency Injection: Ensure ApiClient is implemented as a Singleton (e.g., via Hilt) to prevent memory leaks.
  • Layout Optimization: Replace deep LinearLayout hierarchies with ConstraintLayout to flatten the view hierarchy and improve rendering speed.
  • Testing: Implement unit tests for all new API endpoints and business logic.

🛡️ Security

  • Credential Management: Transition any hardcoded keys to local.properties or the Android Secrets Gradle Plugin.
  • PII Protection: Audit LoginActivity and MyResumesFragment to ensure Personally Identifiable Information (PII) is not being printed to logs (Log.d).
  • Obfuscation: Ensure the new models and API client are included in ProGuard/R8 rules for the release build.
  • Backend Validation: Confirm that the server validates session tokens against resource ownership for the resume retrieval endpoint.

🏗️ Infrastructure & Git Hygiene

  • Repository Cleanup: Run git rm -r --cached .idea and update the project .gitignore immediately.
  • PR Atomicitiy: In the future, split large PRs. This PR mixes a theme change (Teal), a feature change (Resumes), and auth setup. Suggest splitting these into three separate, smaller PRs for easier auditing.

✅ Positive Aspects

  • Comprehensive Documentation: The addition of GOOGLE_AUTH_SETUP.md shows a proactive approach to onboarding other developers to the authentication flow.
  • Consistent Theming: The "teal" overhaul is extensive, covering colors, themes, and attributes, indicating a thorough approach to the UI redesign.
  • Feature Completeness: The PR covers the full vertical slice from API integration to UI fragments.

🏁 Final Checklist for Merge

  • Remove .idea/ files and update .gitignore.
  • Audit GOOGLE_AUTH_SETUP.md and ApiClient.kt for secrets.
  • Provide Unit Tests for new logic/endpoints.
  • Confirm ViewModel implementation for data handling.
  • Verify Backend BOLA protections for the "My Resumes" feature.
  • Verify that no Log.d or println statements remain in production code.
## PR Review Results # Consolidated Review Report: `feature/my-resumes-teal` ## 📋 Executive Summary This pull request introduces the "My Resumes" feature, a comprehensive UI overhaul adopting a "teal" theme, and the integration of Google Authentication. While the functional additions are valuable, the PR currently fails basic version control and security hygiene. The most critical issue is the inclusion of internal IDE configuration files (`.idea/`), which creates technical debt and security risks. Additionally, the introduction of new API endpoints for user-specific data requires verification of backend authorization (BOLA) to prevent data leaks. The lack of unit tests for new business logic is also a significant gap in quality assurance. **Overall Recommendation: 🔴 SIGNIFICANT CHANGES NEEDED** --- ## 🚩 Prioritized Issue List ### 🔴 Critical (Immediate Action Required) * **Version Control Pollution:** Inclusion of `.idea/` folder files. These must be removed and added to `.gitignore` to prevent configuration drift and potential leak of local system metadata. * **Potential Secret Leakage:** `GOOGLE_AUTH_SETUP.md` and `ApiClient.kt` must be audited to ensure no live Client IDs, Secrets, or API keys are committed to source control. ### 🟠 High Priority * **Broken Object Level Authorization (BOLA):** The new "My Resumes" endpoint must be verified on the backend to ensure users cannot access other users' resumes by manipulating IDs in the request. * **Missing Test Coverage:** No unit tests were provided for the new `ApiClient` logic or the `MyResumesFragment` business logic. * **Architectural Concerns:** Potential violation of the Single Responsibility Principle if API logic is implemented directly within Fragments/Activities instead of a `ViewModel`. ### 🟡 Medium Priority * **Resource Management:** Use of hardcoded colors instead of Theme Attributes (`?attr/colorPrimary`), which will hinder future Dark Mode implementation. * **Network Security:** Need to verify the presence of `network_security_config.xml` to enforce HTTPS and disable cleartext traffic. * **UI Performance:** Potential "over-nesting" of layouts in `fragment_job_form.xml` and `activity_login.xml`. ### 🔵 Low Priority * **String Externalization:** Ensure all new strings are in `strings.xml` for localization. * **Model Immutability:** Verify that new data models in `Models.kt` use `val` for thread safety. --- ## 🛠 Domain-Specific Recommendations ### 💻 Code Quality & Architecture * **Decouple UI and Logic:** Move all `ApiClient` calls and data transformations from `LoginActivity` and `MyResumesFragment` into a `ViewModel`. Use `LiveData` or `StateFlow` for UI updates. * **Dependency Injection:** Ensure `ApiClient` is implemented as a Singleton (e.g., via Hilt) to prevent memory leaks. * **Layout Optimization:** Replace deep `LinearLayout` hierarchies with `ConstraintLayout` to flatten the view hierarchy and improve rendering speed. * **Testing:** Implement unit tests for all new API endpoints and business logic. ### 🛡️ Security * **Credential Management:** Transition any hardcoded keys to `local.properties` or the *Android Secrets Gradle Plugin*. * **PII Protection:** Audit `LoginActivity` and `MyResumesFragment` to ensure Personally Identifiable Information (PII) is not being printed to logs (`Log.d`). * **Obfuscation:** Ensure the new models and API client are included in ProGuard/R8 rules for the release build. * **Backend Validation:** Confirm that the server validates session tokens against resource ownership for the resume retrieval endpoint. ### 🏗️ Infrastructure & Git Hygiene * **Repository Cleanup:** Run `git rm -r --cached .idea` and update the project `.gitignore` immediately. * **PR Atomicitiy:** In the future, split large PRs. This PR mixes a theme change (Teal), a feature change (Resumes), and auth setup. Suggest splitting these into three separate, smaller PRs for easier auditing. --- ## ✅ Positive Aspects * **Comprehensive Documentation:** The addition of `GOOGLE_AUTH_SETUP.md` shows a proactive approach to onboarding other developers to the authentication flow. * **Consistent Theming:** The "teal" overhaul is extensive, covering colors, themes, and attributes, indicating a thorough approach to the UI redesign. * **Feature Completeness:** The PR covers the full vertical slice from API integration to UI fragments. --- ## 🏁 Final Checklist for Merge - [ ] **Remove `.idea/` files and update `.gitignore`.** - [ ] **Audit `GOOGLE_AUTH_SETUP.md` and `ApiClient.kt` for secrets.** - [ ] **Provide Unit Tests for new logic/endpoints.** - [ ] **Confirm `ViewModel` implementation for data handling.** - [ ] **Verify Backend BOLA protections for the "My Resumes" feature.** - [ ] **Verify that no `Log.d` or `println` statements remain in production code.**
Opencode closed this pull request 2026-08-04 23:51:01 +10:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
hermes Opencode armistace
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: armistace/resbuilder_app#1
No description provided.
Delete Branch "feature/my-resumes-teal"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?