Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When copying an item, don't add "(copy)". #10404

Merged
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
41 changes: 17 additions & 24 deletions components/api_server/src/model/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,73 +7,66 @@
from shared.model.metric import Metric
from shared.model.source import Source
from shared.utils.type import ItemId
from shared_data_model import DATA_MODEL

from model.report import Report
from utils.functions import uuid
from utils.type import Position


def copy_item(item, **kwargs):
def copy_item[Item: Metric | Report | Source | Subject](item: Item, **kwargs) -> Item:
"""Return a copy of the item."""
item_copy = item.copy()
item_copy = cast(Item, item.copy())
for key, value in kwargs.items():
item_copy[key] = value
return item_copy


def copy_source(source, change_name: bool = True):
def copy_source(source: Source) -> Source:
"""Return a copy of the source."""
kwargs = {}
if change_name:
kwargs["name"] = f"{source.get('name') or DATA_MODEL.sources[source['type']].name} (copy)"
return copy_item(source, **kwargs)
return copy_item(source)


def copy_metric(metric, change_name: bool = True):
def copy_metric(metric: Metric) -> Metric:
"""Return a copy of the metric and its sources."""
kwargs: dict[str, Any] = {
"sources": {uuid(): copy_source(source, change_name=False) for source in metric["sources"].values()},
"sources": {uuid(): copy_source(source) for source in metric["sources"].values()},
}
if change_name:
kwargs["name"] = f"{metric.get('name') or DATA_MODEL.metrics[metric['type']].name} (copy)"
return copy_item(metric, **kwargs)


def copy_subject(subject, change_name: bool = True):
def copy_subject(subject: Subject) -> Subject:
"""Return a copy of the subject, its metrics, and their sources."""
kwargs: dict[str, Any] = {
"metrics": {uuid(): copy_metric(metric, change_name=False) for metric in subject["metrics"].values()},
"metrics": {uuid(): copy_metric(metric) for metric in subject["metrics"].values()},
}
if change_name:
kwargs["name"] = f"{subject.get('name') or DATA_MODEL.all_subjects[subject['type']].name} (copy)"
return copy_item(subject, **kwargs)


def copy_report(report):
def copy_report(report: Report) -> Report:
"""Return a copy of the report, its subjects, their metrics, and their sources."""
return copy_item(
report,
report_uuid=uuid(),
title=f"{report['title']} (copy)",
subjects={uuid(): copy_subject(subject, change_name=False) for subject in report["subjects"].values()},
subjects={uuid(): copy_subject(subject) for subject in report["subjects"].values()},
)


type ItemsDictType = MutableMapping[ItemId, Metric | Source | Subject]


def move_item(
container: Report | Subject | Metric,
item_to_move: Subject | Metric | Source,
new_position: Position,
) -> tuple[int, int]:
"""Change the item position."""
items_dict_type = MutableMapping[ItemId, Metric | Source | Subject]
items_dict: items_dict_type
items_dict: ItemsDictType
if isinstance(container, Report):
items_dict = cast(items_dict_type, container.subjects_dict)
items_dict = cast(ItemsDictType, container.subjects_dict)
elif isinstance(container, Subject):
items_dict = cast(items_dict_type, container.metrics_dict)
items_dict = cast(ItemsDictType, container.metrics_dict)
else:
items_dict = cast(items_dict_type, container.sources_dict)
items_dict = cast(ItemsDictType, container.sources_dict)

nr_items = len(items_dict)
old_index = list(items_dict.keys()).index(item_to_move.uuid)
Expand Down
43 changes: 14 additions & 29 deletions components/api_server/tests/model/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,15 @@ def setUp(self):
self.source = {"name": "Source", "type": "pip"}

def test_copy_name(self):
"""Test that the copy name is changed."""
"""Test that the copy name is not changed."""
source_copy = copy_source(self.source)
self.assertEqual("Source (copy)", source_copy["name"])
self.assertEqual("Source", source_copy["name"])

def test_copy_without_name(self):
"""Test that the copy name is based on the data model if the original source doesn't have a name."""
"""Test that the copy name is not changed."""
self.source["name"] = ""
source_copy = copy_source(self.source)
self.assertEqual("pip (copy)", source_copy["name"])

def test_copy_without_name_change(self):
"""Test that the copy name can be left unchanged."""
source_copy = copy_source(self.source, change_name=False)
self.assertEqual("Source", source_copy["name"])
self.assertEqual("", source_copy["name"])


class CopyMetricTest(unittest.TestCase):
Expand All @@ -43,20 +38,15 @@ def setUp(self):
}

def test_copy_name(self):
"""Test that the copy name is changed."""
"""Test that the copy name is not changed."""
metric_copy = copy_metric(self.metric)
self.assertEqual("Metric (copy)", metric_copy["name"])
self.assertEqual("Metric", metric_copy["name"])

def test_copy_without_name(self):
"""Test that the copy name is based on the data model if the original metric doesn't have a name."""
"""Test that the copy name is not changed."""
self.metric["name"] = ""
metric_copy = copy_metric(self.metric)
self.assertEqual("Security warnings (copy)", metric_copy["name"])

def test_copy_without_name_change(self):
"""Test that the copy name can be left unchanged."""
metric_copy = copy_metric(self.metric, change_name=False)
self.assertEqual("Metric", metric_copy["name"])
self.assertEqual("", metric_copy["name"])

def test_copy_sources(self):
"""Test that the sources are copied too."""
Expand All @@ -76,20 +66,15 @@ def setUp(self):
}

def test_copy_name(self):
"""Test that the copy name is changed."""
"""Test that the copy name is not changed."""
subject_copy = copy_subject(self.subject)
self.assertEqual("Subject (copy)", subject_copy["name"])
self.assertEqual("Subject", subject_copy["name"])

def test_copy_without_name(self):
"""Test that the copy name is based on the data model if the original subject doesn't have a name."""
"""Test that the copy name is not changed."""
self.subject["name"] = ""
subject_copy = copy_subject(self.subject)
self.assertEqual("Software (copy)", subject_copy["name"])

def test_copy_without_name_change(self):
"""Test that the copy name can be left unchanged."""
subject_copy = copy_subject(self.subject, change_name=False)
self.assertEqual("Subject", subject_copy["name"])
self.assertEqual("", subject_copy["name"])

def test_copy_metrics(self):
"""Test that the metrics are copied too."""
Expand All @@ -109,9 +94,9 @@ def setUp(self):
}

def test_copy_title(self):
"""Test that the copy title is changed."""
"""Test that the copy title is not changed."""
report_copy = copy_report(self.report)
self.assertEqual("Report (copy)", report_copy["title"])
self.assertEqual("Report", report_copy["title"])

def test_copy_report_uuid(self):
"""Test that the report UUID can be changed."""
Expand Down
2 changes: 1 addition & 1 deletion components/api_server/tests/routes/test_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ def test_copy_source(self):
copied_source_uuid, copied_source = list(
updated_report["subjects"][SUBJECT_ID]["metrics"][METRIC_ID]["sources"].items(),
)[1]
self.assertEqual("Source (copy)", copied_source["name"])
self.assertEqual("Source", copied_source["name"])
uuids = [REPORT_ID, SUBJECT_ID, METRIC_ID, copied_source_uuid]
description = (
"Jenny copied the source 'Source' of metric 'Metric' of subject 'Subject' from report 'Report' to metric "
Expand Down
4 changes: 4 additions & 0 deletions docs/src/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ If your currently installed *Quality-time* version is not the latest version, pl
- Change the 'unmerged branches' metric to 'inactive branches', also enabling it to count branches that have been merged but not deleted. Closes [#1253](https://github.com/ICTU/quality-time/issues/1253).
- Set the MongoDB feature compatibility version to v8. Closes [#10357](https://github.com/ICTU/quality-time/issues/10357).

### Removed

- When copying a subject, metric, or source, don't add "(copy)" to the name. Closes [#9859](https://github.com/ICTU/quality-time/issues/9859).

## v5.19.0 - 2024-11-22

### Added
Expand Down
5 changes: 2 additions & 3 deletions tests/feature_tests/src/features/metric.feature
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@ Feature: metric
Scenario: copy metric
Given an existing metric
When the client copies the metric
Then the metric name is "Violations (copy)"
Then the subject contains 2 metrics

Scenario: copy metric with source
Given an existing metric
And an existing source
When the client copies the metric
Then the metric name is "Violations (copy)"
And the metric contains 1 source
Then the metric contains 1 source

Scenario: move metric to another report
Given an existing metric
Expand Down
5 changes: 2 additions & 3 deletions tests/feature_tests/src/features/report.feature
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Feature: report
Scenario: copy report
Given an existing report
When the client copies the report
Then the report title is "New report (copy)"
Then the reports overview contains 2 reports

Scenario: copy non-existing report
When the client copies a non-existing report
Expand All @@ -30,8 +30,7 @@ Feature: report
Given an existing report
And an existing subject
When the client copies the report
Then the report title is "New report (copy)"
And the report contains 1 subject
Then the report contains 1 subject

Scenario: change report title
When the client creates a report
Expand Down
2 changes: 1 addition & 1 deletion tests/feature_tests/src/features/source.feature
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Feature: source
Scenario: copy source
Given an existing source
When the client copies the source
Then the source name is "Axe-core (copy)"
Then the metric contains 2 sources

Scenario: move source to another report
Given an existing source
Expand Down
5 changes: 2 additions & 3 deletions tests/feature_tests/src/features/subject.feature
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ Feature: subject
Scenario: copy subject
Given an existing subject
When the client copies the subject
Then the subject name is "Software (copy)"
Then the report contains 2 subjects

Scenario: copy subject with metric
Given an existing subject
And an existing metric
When the client copies the subject
Then the subject name is "Software (copy)"
And the subject contains 1 metric
Then the subject contains 1 metric

Scenario: move subject
Given an existing subject
Expand Down
10 changes: 7 additions & 3 deletions tests/feature_tests/src/steps/item.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ def check_item_order( # noqa: PLR0913
@then("the {container} contains {number} {children}")
def check_nr_children(context: Context, container: str, number: str, children: str) -> None:
"""Check that the container has the expected number of child items."""
container_instance = get_container(context, container)
children = children if children.endswith("s") else children + "s"
assert_equal(number, str(len(container_instance[children])))
if container == "reports overview":
reports = context.get("report/")
assert_equal(number, str(len(reports)))
else:
container_instance = get_container(context, container)
children = children if children.endswith("s") else children + "s"
assert_equal(number, str(len(container_instance[children])))