Add support for SubjectSet metadata as editable attribute

[lcjohnso]

Closes zooniverse/panoptes-cli#256

The issue: when SubjectSet.metadata attribute is edited, there is no check for changes in that field, so it is never added to modified_attributes set and therefore those edits were never pushed as a result of the save() call.

Following same solution as in #222 and #251 for workflow and project attributes:

• add code to initialize current vs original check of metadata dict in set_raw and __init__ functions
• add check to save() to add metadata attribute to modified_attributes if contents are different

Note on change in treatment of SubjectSet.metadata field: I've changed metadata to a standalone key in SubjectSet._edit_attributes field as opposed to dict similar to links field. The distinction between these two treatments of the attribute occur in PanoptesObject._savable_dict() (see https://github.com/zooniverse/panoptes-python-client/blob/master/panoptes_client/panoptes.py#L759-L793), but I'm following the precedent already used by Subject class and elsewhere for these dict attribute fields.

Note on CLI issue: although treatment of SubjectSet metadata has been implemented in the CLI for ~2 years (since zooniverse/panoptes-cli#214), it looks like this feature -- identifying and flagging indexed fields by adding them to indexFields key in SubjectSet metadata -- has never worked up to this point.

This PR currently needs testing:

• check that metadata edits behave as expected
• check that CLI issue is in fact resolved

[eatyourgreens]

Subject set tests need updating somewhere to add in metadata.

File "/home/runner/work/panoptes-python-client/panoptes-python-client/panoptes_client/tests/test_subject_set.py", line 31, in test_create
    pc.client().post.assert_called_with(
  File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/unittest/mock.py", line 907, in assert_called_with
    raise AssertionError(_error_message()) from cause
AssertionError: expected call not found.
Expected: post('/subject_sets', json={'subject_sets': {'display_name': 'Name', 'links': {'project': 1234}}}, etag=None)
Actual: post('/subject_sets', json={'subject_sets': {'display_name': 'Name', 'metadata': {}, 'links': {'project': 1234}}}, etag=None)

[snblickhan]

Commenting so I can get emails about progress of this PR (not a rush, just so I don't forget)

[lcjohnso]

This PR currently has an implementation issue: most SubjectSets do not have any metadata, so therefore metadata == {}. When the SubjectSet object is initialized and set_raw() runs, that means self.metadata will evaluate as False and the elif statement will run (see https://github.com/zooniverse/panoptes-python-client/pull/303/files#diff-26c3a9c285248c88ab6554abda7a5e745a416b746451a546640287adced90766R79-R80) and set _original_metadata to None, causing problems when that variable needs to be evaluated in SubjectSet.save() to check for metadata changes.

This buggy behavior also exists for subjects (and elsewhere), but hasn't been discovered until now because most other resources have non-empty metadata so never encounter this case.

The simple solution would be to always set _original_metadata via deepcopy() as in the current L78 of set_raw(). That seems to work for an empty dict, but perhaps I'm missing why a separate statement for self.metadata = False was originally added. Some testing would be good to do here.

[yuenmichelle1]

Changes look ok. I had some questions and comments. Not blocking.

Going off the fact that you and Toyosi have tested your changes and the want for a new version release, changes look fine to merge.

[yuenmichelle1]

OOC, what was the motivation for this change on Subject.py? Is it related to this change (i.e. fixing issue of allowing subject_set metadata to be editable)?

[lcjohnso]

I used the same pattern related to metadata and _original_metadata in the SubjectSet class as is used for the Subject class. Therefore I applied the same fix to Subjects as was used for SubjectSets. We realized that the "how to initialize with empty metadata" issue affected both classes, but hasn't been reported as a bug for Subjects due to the fact that few subjects are created without metadata.

[yuenmichelle1]

nitpicky, but we can probably test the case where we want metadata to be updated.

something similar to this but with
subject_set.metadata['indexField'] = some index field

to test the case that is mentioned in issue zooniverse/panoptes-cli#256 broken in cli.

Given the state of python client (and the fact that you and Toyosi have mentioned that you all have tested extensively) , this test I guess is not necessary but would be a nice to have.

[lcjohnso]

I'm crap at writing tests. If you and Toyosi can figure out how to write an appropriate test for the with and without metadata cases, please do go ahead and add. However, I don't consider that a requirement for merging this PR -- the current test is basically a unit test, and the adjustment I made was to reflect how existing resources are loaded.