Merge pull request #80 from eReuse/bugfix/79-manual-merge

Bugfix/79 manual merge

ereuse_devicehub/__init__.py

@@ -1 +1 @@
-__version__ = "1.0b"
+__version__ = "1.0.1-beta"

ereuse_devicehub/resources/CHANGELOG.md

@@ -0,0 +1,8 @@
+# Changelog
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
+and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
+
+## [1.0.1-beta] - 2020-11-16
+- [fixed] #80 manual merged from website

ereuse_devicehub/resources/device/definitions.py

@@ -27,11 +27,13 @@ class DeviceDef(Resource):
                          url_prefix, subdomain, url_defaults, root_path, cli_commands)
 
         device_merge = DeviceMergeView.as_view('merge-devices', definition=self, auth=app.auth)
+
         if self.AUTH:
             device_merge = app.auth.requires_auth(device_merge)
-        self.add_url_rule('/<{}:{}>/merge/'.format(self.ID_CONVERTER.value, self.ID_NAME),
+
-                          view_func=device_merge,
+        path = '/<{value}:dev1_id>/merge/<{value}:dev2_id>'.format(value=self.ID_CONVERTER.value)
-                          methods={'POST'})
+
+        self.add_url_rule(path, view_func=device_merge, methods={'POST'})
 
 
 class ComputerDef(DeviceDef):

ereuse_devicehub/resources/device/views.py

@@ -6,10 +6,13 @@ import marshmallow
 from flask import g, current_app as app, render_template, request, Response
 from flask.json import jsonify
 from flask_sqlalchemy import Pagination
+from sqlalchemy.util import OrderedSet
 from marshmallow import fields, fields as f, validate as v, Schema as MarshmallowSchema
 from teal import query
+from teal.db import ResourceNotFound
 from teal.cache import cache
 from teal.resource import View
+from teal.marshmallow import ValidationError
 
 from ereuse_devicehub import auth
 from ereuse_devicehub.db import db
@@ -170,66 +173,78 @@ class DeviceView(View):
 
 class DeviceMergeView(View):
     """View for merging two devices
-    Ex. ``device/<id>/merge/id=X``.
+    Ex. ``device/<dev1_id>/merge/<dev2_id>``.
     """
 
-    class FindArgs(MarshmallowSchema):
+    def post(self, dev1_id: int, dev2_id: int):
-        id = fields.Integer()
+        device = self.merge_devices(dev1_id, dev2_id)
 
-    def get_merge_id(self) -> uuid.UUID:
-        args = self.QUERY_PARSER.parse(self.find_args, request, locations=('querystring',))
-        return args['id']
-
-    def post(self, id: uuid.UUID):
-        device = Device.query.filter_by(id=id).one()
-        with_device = Device.query.filter_by(id=self.get_merge_id()).one()
-        self.merge_devices(device, with_device)
-
-        db.session().final_flush()
         ret = self.schema.jsonify(device)
         ret.status_code = 201
 
         db.session.commit()
         return ret
 
-    def merge_devices(self, base_device, with_device):
+    @auth.Auth.requires_auth
-        """Merge the current device with `with_device` by
+    def merge_devices(self, dev1_id: int, dev2_id: int) -> Device:
-        adding all `with_device` actions under the current device.
+        """Merge the current device with `with_device` (dev2_id) by
+        adding all `with_device` actions under the current device, (dev1_id).
 
         This operation is highly costly as it forces refreshing
         many models in session.
         """
-        snapshots = sorted(
+        # base_device = Device.query.filter_by(id=dev1_id, owner_id=g.user.id).one()
-            filterfalse(lambda x: not isinstance(x, actions.Snapshot), (base_device.actions + with_device.actions)))
+        self.base_device = Device.query.filter_by(id=dev1_id).one()
-        workbench_snapshots = [s for s in snapshots if
+        self.with_device = Device.query.filter_by(id=dev2_id).one()
-                               s.software == (SnapshotSoftware.Workbench or SnapshotSoftware.WorkbenchAndroid)]
-        latest_snapshot_device = [d for d in (base_device, with_device) if d.id == snapshots[-1].device.id][0]
-        latest_snapshotworkbench_device = \
-            [d for d in (base_device, with_device) if d.id == workbench_snapshots[-1].device.id][0]
-        # Adding actions of with_device
-        with_actions_one = [a for a in with_device.actions if isinstance(a, actions.ActionWithOneDevice)]
-        with_actions_multiple = [a for a in with_device.actions if isinstance(a, actions.ActionWithMultipleDevices)]
 
+        if not self.base_device.type == self.with_device.type:
+            # Validation than we are speaking of the same kind of devices
+            raise ValidationError('The devices is not the same type.')
+
+        # Adding actions of self.with_device
+        with_actions_one = [a for a in self.with_device.actions
+                            if isinstance(a, actions.ActionWithOneDevice)]
+        with_actions_multiple = [a for a in self.with_device.actions
+                                 if isinstance(a, actions.ActionWithMultipleDevices)]
+
+        # Moving the tags from `with_device` to `base_device`
+        # Union of tags the device had plus the (potentially) new ones
+        self.base_device.tags.update([x for x in self.with_device.tags])
+        self.with_device.tags.clear()  # We don't want to add the transient dummy tags
+        db.session.add(self.with_device)
+
+        # Moving the actions from `with_device` to `base_device`
         for action in with_actions_one:
             if action.parent:
-                action.parent = base_device
+                action.parent = self.base_device
             else:
-                base_device.actions_one.add(action)
+                self.base_device.actions_one.add(action)
         for action in with_actions_multiple:
             if action.parent:
-                action.parent = base_device
+                action.parent = self.base_device
             else:
-                base_device.actions_multiple.add(action)
+                self.base_device.actions_multiple.add(action)
 
-        # Keeping the components of latest SnapshotWorkbench
+        # Keeping the components of with_device
-        base_device.components = latest_snapshotworkbench_device.components
+        components = OrderedSet(c for c in self.with_device.components)
+        self.base_device.components = components
 
-        # Properties from latest Snapshot
+        # Properties from with_device
-        base_device.type = latest_snapshot_device.type
+        self.merge()
-        base_device.hid = latest_snapshot_device.hid
+
-        base_device.manufacturer = latest_snapshot_device.manufacturer
+        db.session().add(self.base_device)
-        base_device.model = latest_snapshot_device.model
+        db.session().final_flush()
-        base_device.chassis = latest_snapshot_device.chassis
+        return self.base_device
+
+    def merge(self):
+        """Copies the physical properties of the base_device to the with_device.
+        This method mutates base_device.
+        """
+        for field_name, value in self.with_device.physical_properties.items():
+            if value is not None:
+                setattr(self.base_device, field_name, value)
+
+        self.base_device.hid = self.with_device.hid
 
 
 class ManufacturerView(View):

tests/test_basic.py

@@ -30,76 +30,76 @@ def test_api_docs(client: Client):
     assert set(docs['paths'].keys()) == {
         '/actions/',
         '/apidocs',
-        '/batteries/{id}/merge/',
+        '/batteries/{dev1_id}/merge/{dev2_id}',
-        '/bikes/{id}/merge/',
+        '/bikes/{dev1_id}/merge/{dev2_id}',
-        '/cameras/{id}/merge/',
+        '/cameras/{dev1_id}/merge/{dev2_id}',
-        '/cellphones/{id}/merge/',
+        '/cellphones/{dev1_id}/merge/{dev2_id}',
-        '/components/{id}/merge/',
+        '/components/{dev1_id}/merge/{dev2_id}',
-        '/computer-accessories/{id}/merge/',
+        '/computer-accessories/{dev1_id}/merge/{dev2_id}',
-        '/computer-monitors/{id}/merge/',
+        '/computer-monitors/{dev1_id}/merge/{dev2_id}',
-        '/computers/{id}/merge/',
+        '/computers/{dev1_id}/merge/{dev2_id}',
-        '/cookings/{id}/merge/',
+        '/cookings/{dev1_id}/merge/{dev2_id}',
-        '/data-storages/{id}/merge/',
+        '/data-storages/{dev1_id}/merge/{dev2_id}',
-        '/dehumidifiers/{id}/merge/',
+        '/dehumidifiers/{dev1_id}/merge/{dev2_id}',
         '/deliverynotes/',
-        '/desktops/{id}/merge/',
+        '/desktops/{dev1_id}/merge/{dev2_id}',
         '/devices/',
         '/devices/static/{filename}',
-        '/devices/{id}/merge/',
+        '/devices/{dev1_id}/merge/{dev2_id}',
-        '/displays/{id}/merge/',
+        '/displays/{dev1_id}/merge/{dev2_id}',
-        '/diy-and-gardenings/{id}/merge/',
+        '/diy-and-gardenings/{dev1_id}/merge/{dev2_id}',
         '/documents/devices/',
         '/documents/erasures/',
         '/documents/lots/',
         '/documents/static/{filename}',
         '/documents/stock/',
-        '/drills/{id}/merge/',
+        '/drills/{dev1_id}/merge/{dev2_id}',
-        '/graphic-cards/{id}/merge/',
+        '/graphic-cards/{dev1_id}/merge/{dev2_id}',
-        '/hard-drives/{id}/merge/',
+        '/hard-drives/{dev1_id}/merge/{dev2_id}',
-        '/homes/{id}/merge/',
+        '/homes/{dev1_id}/merge/{dev2_id}',
-        '/hubs/{id}/merge/',
+        '/hubs/{dev1_id}/merge/{dev2_id}',
-        '/keyboards/{id}/merge/',
+        '/keyboards/{dev1_id}/merge/{dev2_id}',
-        '/label-printers/{id}/merge/',
+        '/label-printers/{dev1_id}/merge/{dev2_id}',
-        '/laptops/{id}/merge/',
+        '/laptops/{dev1_id}/merge/{dev2_id}',
         '/lots/',
         '/lots/{id}/children',
         '/lots/{id}/devices',
         '/manufacturers/',
-        '/memory-card-readers/{id}/merge/',
+        '/memory-card-readers/{dev1_id}/merge/{dev2_id}',
-        '/mice/{id}/merge/',
+        '/mice/{dev1_id}/merge/{dev2_id}',
-        '/microphones/{id}/merge/',
+        '/microphones/{dev1_id}/merge/{dev2_id}',
-        '/mixers/{id}/merge/',
+        '/mixers/{dev1_id}/merge/{dev2_id}',
-        '/mobiles/{id}/merge/',
+        '/mobiles/{dev1_id}/merge/{dev2_id}',
-        '/monitors/{id}/merge/',
+        '/monitors/{dev1_id}/merge/{dev2_id}',
-        '/motherboards/{id}/merge/',
+        '/motherboards/{dev1_id}/merge/{dev2_id}',
-        '/network-adapters/{id}/merge/',
+        '/network-adapters/{dev1_id}/merge/{dev2_id}',
-        '/networkings/{id}/merge/',
+        '/networkings/{dev1_id}/merge/{dev2_id}',
-        '/pack-of-screwdrivers/{id}/merge/',
+        '/pack-of-screwdrivers/{dev1_id}/merge/{dev2_id}',
-        '/printers/{id}/merge/',
+        '/printers/{dev1_id}/merge/{dev2_id}',
-        '/processors/{id}/merge/',
+        '/processors/{dev1_id}/merge/{dev2_id}',
         '/proofs/',
-        '/rackets/{id}/merge/',
+        '/rackets/{dev1_id}/merge/{dev2_id}',
-        '/ram-modules/{id}/merge/',
+        '/ram-modules/{dev1_id}/merge/{dev2_id}',
-        '/recreations/{id}/merge/',
+        '/recreations/{dev1_id}/merge/{dev2_id}',
-        '/routers/{id}/merge/',
+        '/routers/{dev1_id}/merge/{dev2_id}',
-        '/sais/{id}/merge/',
+        '/sais/{dev1_id}/merge/{dev2_id}',
-        '/servers/{id}/merge/',
+        '/servers/{dev1_id}/merge/{dev2_id}',
-        '/smartphones/{id}/merge/',
+        '/smartphones/{dev1_id}/merge/{dev2_id}',
-        '/solid-state-drives/{id}/merge/',
+        '/solid-state-drives/{dev1_id}/merge/{dev2_id}',
-        '/sound-cards/{id}/merge/',
+        '/sound-cards/{dev1_id}/merge/{dev2_id}',
-        '/sounds/{id}/merge/',
+        '/sounds/{dev1_id}/merge/{dev2_id}',
-        '/stairs/{id}/merge/',
+        '/stairs/{dev1_id}/merge/{dev2_id}',
-        '/switches/{id}/merge/',
+        '/switches/{dev1_id}/merge/{dev2_id}',
-        '/tablets/{id}/merge/',
+        '/tablets/{dev1_id}/merge/{dev2_id}',
         '/tags/',
         '/tags/{tag_id}/device/{device_id}',
-        '/television-sets/{id}/merge/',
+        '/television-sets/{dev1_id}/merge/{dev2_id}',
         '/users/',
         '/users/login/',
-        '/video-scalers/{id}/merge/',
+        '/video-scalers/{dev1_id}/merge/{dev2_id}',
-        '/videoconferences/{id}/merge/',
+        '/videoconferences/{dev1_id}/merge/{dev2_id}',
-        '/videos/{id}/merge/',
+        '/videos/{dev1_id}/merge/{dev2_id}',
-        '/wireless-access-points/{id}/merge/',
+        '/wireless-access-points/{dev1_id}/merge/{dev2_id}',
         '/versions/'
     }
     assert docs['info'] == {'title': 'Devicehub', 'version': '0.2'}

tests/test_merge.py

@@ -0,0 +1,87 @@
+import datetime
+from uuid import UUID
+from flask import g
+
+import pytest
+from ereuse_devicehub.client import Client, UserClient
+from ereuse_devicehub.db import db
+from ereuse_devicehub.devicehub import Devicehub
+from ereuse_devicehub.resources.action import models as m
+from ereuse_devicehub.resources.device import models as d
+from ereuse_devicehub.resources.tag import Tag
+from tests import conftest
+from tests.conftest import file as import_snap
+
+
+@pytest.mark.mvp
+def test_simple_merge(app: Devicehub, user: UserClient):
+    """ Check if is correct to do a manual merge """
+    snapshot1, _ = user.post(import_snap('real-custom.snapshot.11'), res=m.Snapshot)
+    snapshot2, _ = user.post(import_snap('real-hp.snapshot.11'), res=m.Snapshot)
+    pc1_id = snapshot1['device']['id']
+    pc2_id = snapshot2['device']['id']
+
+    with app.app_context():
+        pc1 = d.Device.query.filter_by(id=pc1_id).one()
+        pc2 = d.Device.query.filter_by(id=pc2_id).one()
+        n_actions1 = len(pc1.actions)
+        n_actions2 = len(pc2.actions)
+        action1 = pc1.actions[0]
+        action2 = pc2.actions[0]
+        assert not action2 in pc1.actions
+
+        tag = Tag(id='foo-bar', owner_id=user.user['id'])
+        pc2.tags.add(tag)
+        db.session.add(pc2)
+        db.session.commit()
+
+        components1 = [com for com in pc1.components]
+        components2 = [com for com in pc2.components]
+        components1_excluded = [com for com in pc1.components if not com in components2]
+        assert pc1.hid != pc2.hid
+        assert not tag in pc1.tags
+
+        uri = '/devices/%d/merge/%d' % (pc1_id, pc2_id)
+        result, _ = user.post({'id': 1}, uri=uri, status=201)
+
+        assert pc1.hid == pc2.hid
+        assert action1 in pc1.actions
+        assert action2 in pc1.actions
+        assert len(pc1.actions) == n_actions1 + n_actions2
+        assert set(pc2.components) == set()
+        assert tag in pc1.tags
+        assert not tag in pc2.tags
+
+        for com in components2:
+            assert com in pc1.components
+
+        for com in components1_excluded:
+            assert not com in pc1.components
+
+@pytest.mark.mvp
+def test_merge_two_device_with_differents_tags(app: Devicehub, user: UserClient):
+    """ Check if is correct to do a manual merge of 2 diferents devices with diferents tags """
+    snapshot1, _ = user.post(import_snap('real-custom.snapshot.11'), res=m.Snapshot)
+    snapshot2, _ = user.post(import_snap('real-hp.snapshot.11'), res=m.Snapshot)
+    pc1_id = snapshot1['device']['id']
+    pc2_id = snapshot2['device']['id']
+
+    with app.app_context():
+        pc1 = d.Device.query.filter_by(id=pc1_id).one()
+        pc2 = d.Device.query.filter_by(id=pc2_id).one()
+
+        tag1 = Tag(id='fii-bor', owner_id=user.user['id'])
+        tag2 = Tag(id='foo-bar', owner_id=user.user['id'])
+        pc1.tags.add(tag1)
+        pc2.tags.add(tag2)
+        db.session.add(pc1)
+        db.session.add(pc2)
+        db.session.commit()
+
+        uri = '/devices/%d/merge/%d' % (pc1_id, pc2_id)
+        result, _ = user.post({'id': 1}, uri=uri, status=201)
+
+        assert pc1.hid == pc2.hid
+        assert tag1 in pc1.tags
+        assert tag2 in pc1.tags
+