Skip to content

Conversation

@aviatco
Copy link
Collaborator

@aviatco aviatco commented Jan 11, 2026

✨ Description of new changes

Fix #127

This pull request refactors how the CLI determines when to display the sensitive label prompt, when retrieve item definitions versus metadata-only queries. It introduces a utility function to centralize this logic, updates all relevant code paths to use it, and adds comprehensive tests. This ensures that sensitivity label prompts are only shown when definition data is requested, improving both correctness and maintainability.

Definition retrieval logic centralization:

  • Added a new utility function should_retrieve_definition in fab_cmd_get_utils.py to determine when a query requires retrieving the item's definition, consolidating and clarifying the logic for handling sensitive data.
  • Updated all code paths in fab_fs_get_item.py and fab_fs_get.py to use the new utility function, replacing previous ad-hoc checks and removing direct references to ITEM_METADATA_PROPERTIES. [1] [2]

Code cleanup and simplification:

  • Removed the now-unnecessary ITEM_METADATA_PROPERTIES constant from fab_constant.py, since this information is encapsulated in the new utility function.
  • Refactored imports in affected files to use the new utility function and remove unused imports. [1] [2]

Testing and documentation:

  • Added a comprehensive test suite for should_retrieve_definition covering a wide range of query scenarios and edge cases, ensuring robust and predictable behavior.

Changelog:

  • Updated the changelog to note that the sensitivity label prompt is now displayed only when the item definition is requested.# 📥 Pull Request

@aviatco aviatco requested a review from a team as a code owner January 11, 2026 14:59
may-hartov
may-hartov previously approved these changes Jan 11, 2026
"workspaceId",
"folderId",
"properties.nested.value",
"displayName.localized",
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can remove all of these cases and just keep 1 or 2

Copy link
Contributor

@ohadedry ohadedry left a comment

Choose a reason for hiding this comment

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

Please update description to reflect latest changes

@@ -0,0 +1,6 @@
kind: fixed
body: display sensitivity label prompt only when item definition is requested
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the comment to reflect the changes

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.

[BUG] Get item's properties should not display definition prompt

4 participants