Skip to content

Conversation

@SaintPatrck
Copy link
Contributor

🎟️ Tracking

PM-29947

📔 Objective

Remove the shouldResetMasterPassword field (mapped to "ResetMasterPassword") from the GetTokenResponseJson network model. This property is no longer processed or required in the identity service response handling.

  • Delete the shouldResetMasterPassword property and its serialization annotation from the GetTokenResponseJson data class.
  • Update unit tests in AuthRepositoryTest, GetTokenResponseExtensionsTest, and IdentityServiceTest to remove the shouldResetMasterPassword argument from the GetTokenResponseJson constructor calls.

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

Remove the `shouldResetMasterPassword` field (mapped to "ResetMasterPassword") from the `GetTokenResponseJson` network model. This property is no longer processed or required in the identity service response handling.

- Delete the `shouldResetMasterPassword` property and its serialization annotation from the `GetTokenResponseJson` data class.
- Update unit tests in `AuthRepositoryTest`, `GetTokenResponseExtensionsTest`, and `IdentityServiceTest` to remove the `shouldResetMasterPassword` argument from the `GetTokenResponseJson` constructor calls.
@SaintPatrck SaintPatrck marked this pull request as ready for review December 19, 2025 14:49
@SaintPatrck SaintPatrck added the ai-review Request a Claude code review label Dec 19, 2025
@claude
Copy link
Contributor

claude bot commented Dec 19, 2025

Claude finished @SaintPatrck's task in 2m 16s —— View job


Code Review Complete ✓

  • Analyzing PR changes
  • Reviewing code quality and adherence to standards
  • Posting review comments

Overall Assessment: APPROVE

This PR correctly removes the unused shouldResetMasterPassword property from the network model. The property was never actually used in the codebase - the password reset logic uses shouldForcePasswordReset instead. All tests have been properly updated, and the documentation has been cleaned up.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2025

Logo
Checkmarx One – Scan Summary & Details2bbfdf01-cb73-4638-8184-11f2f8b1e900

Great job! No new security vulnerabilities introduced in this pull request

andrebispo5
andrebispo5 previously approved these changes Dec 19, 2025
Remove the outdated documentation for the `shouldResetMasterPassword` property in `GetTokenResponseJson`.

The `shouldResetMasterPassword` property is no longer present in the `GetTokenResponseJson` data class, but its KDoc documentation remained. This change cleans up the KDoc to accurately reflect the available properties.
@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.51%. Comparing base (913d877) to head (9635b3c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6285   +/-   ##
=======================================
  Coverage   85.51%   85.51%           
=======================================
  Files         762      762           
  Lines       54530    54530           
  Branches     7833     7833           
=======================================
  Hits        46630    46630           
  Misses       5183     5183           
  Partials     2717     2717           

☔ 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.

@SaintPatrck SaintPatrck added this pull request to the merge queue Dec 19, 2025
Merged via the queue into main with commit 0741584 Dec 19, 2025
15 checks passed
@SaintPatrck SaintPatrck deleted the PM-29947/remove-reset-master-password-property branch December 19, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants