Skip to content

[FEATURE REQUEST] Add account information to the user#4647

Closed
joragua wants to merge 6 commits intomasterfrom
feature/add_account_id_to_the_user
Closed

[FEATURE REQUEST] Add account information to the user#4647
joragua wants to merge 6 commits intomasterfrom
feature/add_account_id_to_the_user

Conversation

@joragua
Copy link
Collaborator

@joragua joragua commented Jul 23, 2025

Related Issues

App: #4605

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

@joragua joragua self-assigned this Jul 23, 2025
@joragua joragua linked an issue Jul 23, 2025 that may be closed by this pull request
14 tasks
@joragua joragua force-pushed the feature/add_account_id_to_the_user branch 2 times, most recently from 93659ba to 6ff4c9d Compare July 23, 2025 15:30
@joragua joragua force-pushed the feature/add_account_id_to_the_user branch 2 times, most recently from fd03a50 to 447dfc0 Compare August 6, 2025 07:51
@joragua joragua force-pushed the feature/add_account_id_to_the_user branch from 064495d to 1aabf85 Compare August 6, 2025 08:07
@joragua joragua marked this pull request as ready for review August 6, 2025 08:16
@joragua joragua requested a review from jesmrec August 12, 2025 11:40
@@ -0,0 +1,7 @@
Enhancement: Add account ID information to the user
Copy link
Collaborator

Choose a reason for hiding this comment

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

My option here: Add account ID to the user information . User is confusing... it's the one that uses the app :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. Changed! 👍🏻


Timber.d("Get user id completed and parsed to ${result.data}")
} else {
result = RemoteOperationResult(getMethod)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why getMethod as parameter here? shouldn't be any error information like ResultCode.xxx?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The constructor used here takes a HttpMethod as its parameter. It is intended for cases when the result needs to be interpreted from the response (You can see the implementation in the RemoteOperationResult class)

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 . RemoteOperationResult has a bunch of creators calling themselves... that confused me. That's OK

@joragua joragua force-pushed the feature/add_account_id_to_the_user branch from 5453ec9 to 3ac5ccc Compare August 12, 2025 15:30
@jesmrec jesmrec self-requested a review August 12, 2025 15:40
@jesmrec
Copy link
Collaborator

jesmrec commented Aug 12, 2025

CR approved

@jesmrec
Copy link
Collaborator

jesmrec commented Aug 12, 2025

@joragua, the process of adding the client ID works fine for new accounts. But, i guess that it won't work for migrations, since the id is saved in first instance.

@jesmrec jesmrec closed this Aug 12, 2025
@jesmrec jesmrec reopened this Aug 12, 2025
@joragua joragua closed this Aug 14, 2025
@joragua joragua deleted the feature/add_account_id_to_the_user branch August 27, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants