Allow a name to be removed from nro name_instance table (#512)

* Allow a name to be removed from nro name_instance table

* Also delete the missing name from  Namex npostgres db

* remove unnecessary change

* Adjust where names are deleted

api/namex/resources/requests.py

@@ -687,6 +687,7 @@ def put(nr, *args, **kwargs):
             is_changed__name1 = False
             is_changed__name2 = False
             is_changed__name3 = False
+            deleted_names = [False] * 3
 
             if len(nrd.names.all()) == 0:
                 new_name_choice = Name()
@@ -762,13 +763,18 @@ def put(nr, *args, **kwargs):
                                                                     .format(orig_name['name'], nrd_name.name)})
                             if nrd_name.choice == 2:
                                 is_changed__name2 = True
+                                if not nrd_name.name:
+                                    deleted_names[nrd_name.choice - 1] = True
                                 json_input['comments'].append({'comment': 'Name choice 2 changed from {0} to {1}'\
                                                                     .format(orig_name['name'], nrd_name.name)})
                             if nrd_name.choice == 3:
                                 is_changed__name3 = True
+                                if not nrd_name.name:
+                                    deleted_names[nrd_name.choice - 1] = True
                                 json_input['comments'].append({'comment': 'Name choice 3 changed from {0} to {1}'\
                                                                     .format(orig_name['name'], nrd_name.name)})
 
+
             ### END names ###
 
             ### COMMENTS ###
@@ -866,7 +872,6 @@ def put(nr, *args, **kwargs):
                 if warnings:
                     MessageServices.add_message(MessageServices.ERROR, 'change_request_in_NRO', warnings)
 
-
             ### Update NR Details in NRO (not for reset)
             else:
                 try:
@@ -888,6 +893,11 @@ def put(nr, *args, **kwargs):
                         warnings = nro.change_nr(nrd, change_flags)
                         if warnings:
                             MessageServices.add_message(MessageServices.ERROR, 'change_request_in_NRO', warnings)
+                        else:
+                            ### now it's safe to delete any names that were blanked out
+                            for nrd_name in nrd.names:
+                                if deleted_names[nrd_name.choice - 1]:
+                                    nrd_name.delete_from_db()
 
                 except (NROServicesError, Exception) as err:
                     MessageServices.add_message('error', 'change_request_in_NRO', err)

api/namex/services/nro/change_nr.py

@@ -17,7 +17,7 @@
 from namex.models import State
 
 
-def update_nr (nr, ora_cursor, change_flags):
+def update_nr(nr, ora_cursor, change_flags):
     """Update the Name Request in NRO
     :raises Exception: what ever error we get, let our caller handle, this is here in case we want to wrap it - future
     """
@@ -171,7 +171,8 @@ def _update_request(oracle_cursor, nr, event_id, change_flags):
 
 def _update_nro_names(oracle_cursor, nr, event_id, change_flags):
     """find the current name instance, set it's end_event_id to event_id
-       create a new name_instance and set it start_event_id to event_id
+       if the name was deleted, nothing more needs to be done.
+       otherwise, create a new name_instance and set its start_event_id to event_id
     """
 
     for name in nr.names.all():
@@ -228,15 +229,17 @@ def _update_nro_names(oracle_cursor, nr, event_id, change_flags):
                                       name_id=n_id,
                                       start_event=event_id)
 
-            oracle_cursor.execute("""
-            INSERT INTO name_instance (name_instance_id, name_id, choice_number, name, start_event_id, search_name)
-            VALUES (name_instance_seq.nextval, :name_id, :choice, :name, :event_id, :search_name)
-            """,
-                                  name_id=n_id,
-                                  choice=name.choice,
-                                  name=name.name,
-                                  event_id=event_id,
-                                  search_name=generate_compressed_name(name.name))
+            # If the new name is not blank, do this:
+            if name.name:
+                oracle_cursor.execute("""
+                INSERT INTO name_instance (name_instance_id, name_id, choice_number, name, start_event_id, search_name)
+                VALUES (name_instance_seq.nextval, :name_id, :choice, :name, :event_id, :search_name)
+                """,
+                                      name_id=n_id,
+                                      choice=name.choice,
+                                      name=name.name,
+                                      event_id=event_id,
+                                      search_name=generate_compressed_name(name.name))
 
 
 def _update_nro_address(oracle_cursor, nr, event_id, change_flags):

api/tests/python/end_points/test_requests.py

@@ -1,6 +1,5 @@
 from flask import jsonify
-from unittest import mock
-import sys
+from flask import json
 from namex.models import User
 
 token_header = {
@@ -211,3 +210,80 @@ def test_put_nr_view_only(client, jwt, app):
     assert 401 == rv.status_code
     assert expected_response == rv.data
 
+
+def test_add_new_name_to_nr(client, jwt, app):
+
+    # add NR to database
+    from namex.models import Request as RequestDAO, State, Name as NameDAO
+    nr = RequestDAO()
+    nr.nrNum = 'NR 0000002'
+    nr.stateCd = State.INPROGRESS
+    nr.requestId = 1460775
+    name1 = NameDAO()
+    name1.choice = 1
+    name1.name = 'ONE'
+    nr.names = [name1]
+    nr.save_to_db()
+
+    # create JWT & setup header with a Bearer Token using the JWT
+    token = jwt.create_jwt(claims, token_header)
+    headers = {'Authorization': 'Bearer ' + token, 'content-type': 'application/json'}
+
+    # get the resource so we have a template for the request:
+    rv = client.get('/api/v1/requests/NR%200000002', headers=headers)
+    assert rv.status_code == 200
+    # assert we're starting with just one name:
+    data = json.loads(rv.data)
+    assert len(data['names']) == 1
+
+    new_name = data['names'][0]
+    new_name['name'] = 'Name 2'
+    new_name['choice'] = 2
+    data['names'].append(new_name)
+
+    # Update with a brand new name (this is the test)
+    rv = client.put('/api/v1/requests/NR%200000002', data=json.dumps(data), headers=headers)
+
+    data = json.loads(rv.data)
+    assert 200 == rv.status_code
+    assert len(data['names']) == 2
+
+
+def test_remove_name_from_nr(client, jwt, app):
+
+    # add NR to database
+    from namex.models import Request as RequestDAO, State, Name as NameDAO
+    nr = RequestDAO()
+    nr.nrNum = 'NR 0000002'
+    nr.stateCd = State.INPROGRESS
+    nr.requestId = 1460775
+    name1 = NameDAO()
+    name1.choice = 1
+    name1.name = 'ONE'
+    name2 = NameDAO()
+    name2.choice = 2
+    name2.name = 'TWO'
+    nr.names = [name1, name2]
+    nr.save_to_db()
+
+    # create JWT & setup header with a Bearer Token using the JWT
+    token = jwt.create_jwt(claims, token_header)
+    headers = {'Authorization': 'Bearer ' + token, 'content-type': 'application/json'}
+
+    # get the resource so we have a template for the request:
+    rv = client.get('/api/v1/requests/NR%200000002', headers=headers)
+    assert rv.status_code == 200
+    # assert we're starting with just one name:
+    data = json.loads(rv.data)
+    assert len(data['names']) == 2
+
+    for name in data['names'] :
+        if name['choice'] == 2:
+            name['name'] = ''
+
+    # Update with one blank name name (should remove the blank name)
+    rv = client.put('/api/v1/requests/NR%200000002', data=json.dumps(data), headers=headers)
+
+    data = json.loads(rv.data)
+    assert 200 == rv.status_code
+    assert len(data['names']) == 1

api/tests/python/nro_services/test_update_request.py

@@ -3,6 +3,7 @@
 from namex.models import User
 from tests.python import integration_oracle_namesdb, integration_oracle_local_namesdb
 from namex.services.nro.change_nr import \
+    _update_nro_names, \
     _update_request, \
     _get_event_id, \
     _create_nro_transaction, \
@@ -16,9 +17,25 @@ def test_nro_connection(app):
     assert type(conn) is cx_Oracle.Connection
 
 
+class NamesList:
+    mynames = []
+    def addNames(self, names):
+        self.mynames = names
+
+    def all(self):
+        return iter(self.mynames)
+
+
 class FakeRequest:
     requestId = '42'
     previousRequestId = '15'
+    names = NamesList();
+
+
+class FakeName:
+    nameId = '42'
+    name = ''
+    choice = 1
 
 
 @integration_oracle_namesdb
@@ -137,3 +154,130 @@ def test_update_nro_request_state_to_approved(app):
     assert resultset is None
 
 
+@integration_oracle_namesdb
+def test_update_nro_change_and_remove_name_choices(app):
+    """
+    Ensure name can be changed, or removed.
+    """
+    con = nro.connection
+    cursor = con.cursor()
+
+    eid = _get_event_id(cursor)
+
+    user = User('idir/bob', 'bob', 'last', 'idir', 'localhost')
+
+    fake_request = FakeRequest()
+    fake_name1 = FakeName()
+    fake_name2 = FakeName()
+    fake_name3 = FakeName()
+
+    fake_name1.choice = 1
+    fake_name2.choice = 2
+    fake_name3.choice = 3
+    # Update the second name only, and remove the third:
+    fake_name1.name = "Fake name"
+    fake_name2.name = 'Second fake name'
+    fake_name3.name = ''
+    names = NamesList()
+    names.addNames([fake_name1, fake_name2, fake_name3])
+    fake_request.names = names
+
+    fake_request.requestId = 142729
+    fake_request.stateCd = 'INPROGRESS'
+    fake_request.activeUser = user
+
+    change_flags = {
+        'is_changed__name1': False,
+        'is_changed__name2': True,
+        'is_changed__name3': True,
+    }
+
+    # Fail if our test data is not still valid:
+    cursor.execute("""
+        select ni.name
+        from name n, name_instance ni
+        where  ni.name_id = n.name_id 
+        and n.request_id = {} 
+        and ni.end_event_id is null """
+                   .format(fake_request.requestId))
+    result = list(cursor.fetchall())
+    assert len(result) == 3
+    assert (result[1][0] != 'Second fake name')
+
+    _update_nro_names(cursor, fake_request, eid, change_flags)
+
+    cursor.execute("""
+        select ni.name
+        from name n, name_instance ni
+        where  ni.name_id = n.name_id 
+        and n.request_id = {} 
+        and ni.end_event_id is null """
+                   .format(fake_request.requestId))
+    result = list(cursor.fetchall())
+
+    assert result
+    assert len(result) == 2
+    assert result[0][0] != 'Fake name'
+    assert result[1][0] == 'Second fake name'
+
+
+@integration_oracle_namesdb
+def test_update_nro_add_new_name_choice(app):
+    """
+    Ensure name can be changed, or removed.
+    """
+    con = nro.connection
+    cursor = con.cursor()
+
+    eid = _get_event_id(cursor)
+
+    user = User('idir/bob', 'bob', 'last', 'idir', 'localhost')
+
+    fake_request = FakeRequest()
+    fake_name1 = FakeName()
+    fake_name2 = FakeName()
+
+    fake_name1.choice = 1
+    fake_name2.choice = 2
+    # Add a second name choice:
+    fake_name1.name = "Fake name"
+    fake_name2.name = 'Second fake name'
+    names = NamesList()
+    names.addNames([fake_name1, fake_name2])
+    fake_request.names = names
+
+    fake_request.requestId = 884047
+    fake_request.stateCd = 'INPROGRESS'
+    fake_request.activeUser = user
+
+    change_flags = {
+        'is_changed__name1': False,
+        'is_changed__name2': True,
+        'is_changed__name3': False,
+    }
+
+    # Fail if our test data is not still valid:
+    cursor.execute("""
+        select ni.name
+        from name n, name_instance ni
+        where  ni.name_id = n.name_id 
+        and n.request_id = {} 
+        and ni.end_event_id is null """
+                   .format(fake_request.requestId))
+    result = list(cursor.fetchall())
+    assert len(result) == 1
+
+    _update_nro_names(cursor, fake_request, eid, change_flags)
+
+    cursor.execute("""
+        select ni.name
+        from name n, name_instance ni
+        where  ni.name_id = n.name_id 
+        and n.request_id = {} 
+        and ni.end_event_id is null """
+                   .format(fake_request.requestId))
+    result = list(cursor.fetchall())
+
+    assert result
+    assert len(result) == 2
+    assert result[0][0] != 'Fake name'