Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/fixed-20260419-105923.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: fixed
body: Remove hardcoded description when create/import items
time: 2026-04-19T10:59:23.659719244Z
custom:
Author: aviatco
AuthorLink: https://github.com/aviatco
1 change: 0 additions & 1 deletion src/fabric_cli/commands/fs/impor/fab_fs_import_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ def _import_create_environment_item(

item_payload: dict = {
"type": str(item.item_type),
"description": "Imported from fab",
"displayName": item.short_name,
"folderId": item.folder_id,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ def exec(connection: VirtualWorkspaceItem, args: Namespace) -> None:

# Base payload
payload = {
"description": "Created by fab",
"displayName": connection.short_name,
"connectivityType": connectivityType,
}
Expand Down
1 change: 0 additions & 1 deletion src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_folder.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ def exec(folder: Folder, args: Namespace) -> str | None:
utils_ui.print_grey(f"Creating a new Folder...")

payload = {
"description": "Created by fab",
"displayName": foldername,
}
if parent_folder_id:
Expand Down
1 change: 0 additions & 1 deletion src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ def exec(gateway: VirtualWorkspaceItem, args: Namespace) -> None:
}

payload = {
# "description": "Created by fab",
"displayName": gateway.short_name,
"capacityId": params.get("capacityid"),
"inactivityMinutesBeforeSleep": params.get("inactivityminutesbeforesleep", 30),
Expand Down
1 change: 0 additions & 1 deletion src/fabric_cli/commands/fs/mkdir/fab_fs_mkdir_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ def exec(item: Item, args: Namespace) -> str | None:
utils.remove_keys_from_dict(params, ["displayname", "type"])

payload = {
"description": "Created by fab",
"displayName": item_name,
"type": str(item_type),
"folderId": item.folder_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ def exec(workspace: Workspace, args: Namespace) -> None:
utils.remove_keys_from_dict(args.params, ["displayName"])

payload = {
"description": "Created by fab",
"displayName": workspace.short_name,
}
payload.update(args.params)
Expand Down
1 change: 0 additions & 1 deletion src/fabric_cli/utils/fab_cmd_import_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ def get_payload_for_item_type(
definition = _build_definition(path, input_format)
return {
"type": str(item.item_type),
"description": "Imported from fab",
"folderId": item.folder_id,
"displayName": item.short_name,
"definition": definition,
Expand Down
50 changes: 43 additions & 7 deletions tests/test_core/test_fab_hiearchy.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,6 @@ def _mock_build(path, resolved_format=""):

_expected_payload = {
"type": "Notebook",
"description": "Imported from fab",
"displayName": "item_name",
"folderId": None,
"definition": {"format": "ipynb", "parts": _base_payload["parts"]},
Expand All @@ -429,7 +428,6 @@ def _mock_build(path, resolved_format=""):

_expected_payload = {
"type": "SparkJobDefinition",
"description": "Imported from fab",
"displayName": "item_name",
"folderId": None,
"definition": {
Expand All @@ -445,7 +443,6 @@ def _mock_build(path, resolved_format=""):

_expected_payload = {
"type": "SparkJobDefinition",
"description": "Imported from fab",
"displayName": "item_name",
"folderId": None,
"definition": {
Expand All @@ -469,7 +466,6 @@ def _mock_build(path, resolved_format=""):

_expected_payload = {
"type": "Eventhouse",
"description": "Imported from fab",
"displayName": "item_name",
"folderId": None,
"definition": {"parts": _base_payload["parts"]},
Expand All @@ -490,7 +486,6 @@ def _mock_build(path, resolved_format=""):

_expected_payload = {
"type": "Report",
"description": "Imported from fab",
"displayName": "item_name",
"folderId": None,
"definition": {"parts": _base_payload["parts"]},
Expand All @@ -506,7 +501,6 @@ def _mock_build(path, resolved_format=""):

_expected_payload_without_format = {
"type": "SemanticModel",
"description": "Imported from fab",
"displayName": "item_name",
"folderId": None,
"definition": {"parts": _base_payload["parts"]},
Expand All @@ -518,7 +512,6 @@ def _mock_build(path, resolved_format=""):

_expected_payload_with_format = {
"type": "SemanticModel",
"description": "Imported from fab",
"displayName": "item_name",
"folderId": None,
"definition": {
Expand All @@ -538,6 +531,49 @@ def _mock_build(path, resolved_format=""):
assert get_payload_for_item_type("dummy", report) == _expected_payload


def test_import_payload_does_not_contain_description_by_default():
"""Verify that get_payload_for_item_type never stamps a description in the payload
unless the caller explicitly includes one — i.e. the fix for hardcoded 'Imported from fab'."""
tenant = Tenant(name="tenant_name", id="0000")
workspace = Workspace(
name="workspace_name", id="workspace_id", parent=tenant, type="Workspace"
)

def _mock_build(path, resolved_format=""):
result = {"parts": {}}
if resolved_format:
result["format"] = resolved_format
return result

item_types = [
"Notebook",
"SparkJobDefinition",
"EventHouse",
"Report",
"SemanticModel",
"DataPipeline",
"Lakehouse",
]

for item_type in item_types:
item = Item(
name="my_item",
id="item_id",
parent=workspace,
item_type=item_type,
)
with patch(
"fabric_cli.utils.fab_cmd_import_utils._build_definition",
side_effect=_mock_build,
):
payload = get_payload_for_item_type("dummy", item)

assert "description" not in payload, (
f"Payload for {item_type} must not contain 'description' by default, "
f"got: {payload}"
)


# -------------------------------------------------------------------
# Tests for _build_definition format handling
# -------------------------------------------------------------------
Expand Down
80 changes: 77 additions & 3 deletions tests/test_utils/test_fab_cmd_mkdir_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ def test_fabric_data_pipelines_workspace_identity_no_params_success():
"""Test FabricDataPipelines with WorkspaceIdentity credential type when no parameters are required."""
# Arrange
payload = {
"description": "Created by fab",
"displayName": "test-connection",
"connectivityType": "ShareableCloud"
}
Expand Down Expand Up @@ -63,7 +62,6 @@ def test_connection_with_required_params_missing_failure():
"""Test that connection creation fails when required parameters are missing."""
# Arrange
payload = {
"description": "Created by fab",
"displayName": "test-connection",
"connectivityType": "ShareableCloud"
}
Expand Down Expand Up @@ -107,7 +105,6 @@ def test_workspace_identity_with_unsupported_params_ignored_success():
"""Test that WorkspaceIdentity ignores unsupported credential parameters with warning."""
# Arrange
payload = {
"description": "Created by fab",
"displayName": "test-connection",
"connectivityType": "ShareableCloud"
}
Expand Down Expand Up @@ -147,6 +144,83 @@ def test_workspace_identity_with_unsupported_params_ignored_success():
assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity"


def test_connection_payload_does_not_contain_description_by_default_success():
"""Verify that get_connection_config_from_params never stamps a description
in the returned payload when the caller does not supply one."""
payload = {
"displayName": "test-connection",
"connectivityType": "ShareableCloud",
}

con_type = "FabricDataPipelines"
con_type_def = {
"type": "FabricDataPipelines",
"creationMethods": [
{
"name": "FabricDataPipelines.Actions",
"parameters": [],
}
],
"supportedCredentialTypes": ["WorkspaceIdentity"],
}

params = {
"connectiondetails": {
"type": "FabricDataPipelines",
"creationmethod": "FabricDataPipelines.Actions",
},
"credentialdetails": {
"type": "WorkspaceIdentity",
},
}

result = get_connection_config_from_params(
payload, con_type, con_type_def, params)

assert "description" not in result, (
f"Connection payload must not contain 'description' by default, got: {result}"
)


def test_connection_payload_contains_description_when_user_provides_it_success():
"""Verify that when a user explicitly passes description via params it is
forwarded into the payload returned by get_connection_config_from_params."""
payload = {
"displayName": "test-connection",
"connectivityType": "ShareableCloud",
"description": "My custom description", # user supplied before calling helper
}

Comment on lines +185 to +193
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

This test claims the user provides description “via params”, but it injects description directly into the input payload instead. That doesn’t verify the actual CLI path (where description would arrive in args.params) and will still pass even if mkdir connection continues to ignore description. Move the description into params (and/or adjust the docstring) so the test validates param forwarding behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

con_type = "FabricDataPipelines"
con_type_def = {
"type": "FabricDataPipelines",
"creationMethods": [
{
"name": "FabricDataPipelines.Actions",
"parameters": [],
}
],
"supportedCredentialTypes": ["WorkspaceIdentity"],
}

params = {
"connectiondetails": {
"type": "FabricDataPipelines",
"creationmethod": "FabricDataPipelines.Actions",
},
"credentialdetails": {
"type": "WorkspaceIdentity",
},
}

result = get_connection_config_from_params(
payload, con_type, con_type_def, params)

assert result.get("description") == "My custom description", (
f"Connection payload must preserve a user-supplied description, got: {result}"
)
Comment on lines +185 to +221
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

This test claims that a user supplies description “via params”, but the test actually injects description directly into the payload before calling get_connection_config_from_params. That doesn’t exercise the real CLI path (where description would come from args.params) and will keep passing even if the CLI continues to ignore description. Update the test to provide description in params (and/or adjust the docstring) so it verifies the intended behavior end-to-end for param handling.

Copilot generated this review using guidance from repository custom instructions.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback



class TestFindMpeConnection:
"""Test cases for find_mpe_connection function."""

Expand Down
Loading