FLOW-1758: Add attribute to track RDS/RDE paid or unpaid seat in feedback collector - (Rovo Dev)#1756
Conversation
Add user entitlement type (RDS/RDE paid or unpaid seat) to the feedback collector context in RovoDevFeedbackManager. This enables better triage and analysis of feedback by seat type. The entitlement type is fetched asynchronously from the entitlement checker and included in the feedback context. Tests are added to verify inclusion of entitlement type for entitled, non-entitled, and error cases.
Blastoplex
requested review from
BHulovatyi,
amarg-at,
bwieger-atlassian-com,
cabella-dot,
ccallcottstevens,
cindy-atl,
dchiew-atl,
jwang19-atlassian,
marcomura,
matt-lassian,
mattcolman,
om-ukr,
sdzh-atlassian,
sky-ocean-spirits and
teg-atlassian
as code owners
…rde-paid-or-unpaid-seat-a-user-is-on-via-feedb
![atlassian[bot]](https://avatars.githubusercontent.com/in/14320?s=60&v=4){kind=small}
| const entitlement = await Container.rovoDevEntitlementChecker.checkEntitlement(); | ||
| entitlementType = entitlement.type; | ||
| } catch { | ||
| entitlementType = 'unknown'; |
There was a problem hiding this comment.
🔎 Code Readability
The variable assignment on line 109 is redundant since entitlementType is already initialized to 'unknown' on line 104.
Details
📖 Explanation: Removing the redundant assignment in the catch block would simplify the code since the variable is already initialized with the fallback value.
| entitlementType = 'unknown'; | |
| // Intentionally leave entitlementType as 'unknown' when the entitlement check fails |
Remove all instances of the "peer": true field from package-lock.json entries. This cleanup reduces unnecessary metadata clutter in the lock file without affecting package resolution or functionality.
…OW-1758-add-attribute-to-include-what-rds-rde-paid-or-unpaid-seat-a-user-is-on-via-feedb
|
The issue is ready for review. The below acceptance criteria have not been met:
Code Reviewer could not determine whether the following acceptance criteria have been met:
|
| const entitlement = await Container.rovoDevEntitlementChecker.checkEntitlement(); | ||
| entitlementType = entitlement.type; | ||
| } catch { | ||
| entitlementType = 'unknown'; |
There was a problem hiding this comment.
🔎 Code Readability
The error handling in the catch block reassigns entitlementType to 'unknown' which is redundant since it's already initialized to 'unknown'.
Details
📖 Explanation: Removing the redundant assignment in the catch block simplifies the code since the variable is already initialized with the fallback value, making the error handling more concise.
| entitlementType = 'unknown'; | |
| // entitlementType remains 'unknown' if the entitlement check fails |
| fields: expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| id: 'customfield_10047', | ||
| value: expect.stringContaining('"entitlementType": "ROVO_DEV_EVERYWHERE"'), |
There was a problem hiding this comment.
🔎 Testing
The test assertion uses expect.stringContaining to check for JSON content, but this approach is fragile and could match partial strings incorrectly.
Details
📖 Explanation: Using JSON.parse on the value and then checking the parsed object's properties would be more robust and less prone to false positives than string matching.
2 participants
Rovo Dev code review: Rovo Dev is reviewing this pull request…
Refresh the page in a few minutes to see the results.