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

[TASK-957] Add mongo_uuid field to XForm model and populate it #5196

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

rajpatel24
Copy link
Contributor

@rajpatel24 rajpatel24 commented Oct 24, 2024

Checklist

  1. If you've added code that should be tested, add tests
  2. If you've changed APIs, update (or create!) the documentation
  3. Ensure the tests pass
  4. Run ./python-format.sh to make sure that your code lints and that you've followed our coding style
  5. Write a title and, if necessary, a description of your work suitable for publishing in our release notes
  6. Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE)
  7. Open an issue in the docs if there are UI/UX changes
  8. Create a testing plan for the reviewer and add it to the Testing section

Description

This PR introduces a new field, mongo_uuid, to the XForm model in KoboToolbox. This unique identifier is designed to enhance the ownership transfer process of XForms by ensuring that each form is linked to a distinct UUID in MongoDB. The addition of mongo_uuid will streamline how XForms are managed, especially during ownership transfers, by reducing unnecessary updates to the MongoDB documents related to the XForm.

Notes

  • A new column (mongo_uuid) has been added to the XForm model, which will serve as a unique identifier.
    Ownership Transfer: The logic for transferring an XForm's ownership has been updated to ensure that the mongo_uuid is populated if it is currently null.
  • All existing MongoDB documents that match the _userform_id will be updated to include the new mongo_uuid value, ensuring consistency across the database.
  • The ParsedInstance.to_dict_for_mongo() method has been modified to prioritize the mongo_uuid over _userform_id when exporting data. If mongo_uuid is not null, _userform_id will be set to mongo_uuid; otherwise, the original logic will be maintained.
  • The mongo_userform_id property in the OpenRosaDeploymentBackend class has been updated to return the mongo_uuid when available, ensuring that the correct ID is used during operations.

@rajpatel24 rajpatel24 changed the title Add mongo_uuid field to XForm model and populate it Add mongo_uuid field to XForm model and populate it Oct 24, 2024
@rajpatel24 rajpatel24 force-pushed the task-957-implement-mongo-owner-form-id branch from 80734f0 to 47abf96 Compare October 24, 2024 20:19
@rajpatel24 rajpatel24 changed the title Add mongo_uuid field to XForm model and populate it [TASK-957] Add mongo_uuid field to XForm model and populate it Oct 24, 2024
Copy link

@noliveleger noliveleger self-assigned this Oct 25, 2024
@@ -500,17 +501,79 @@ def test_data_accessible_to_new_user(self):
for attachment in response.data['results'][0]['_attachments']:
assert attachment['filename'].startswith('anotheruser/')

# Get the mongo_uuid for the transferred asset (XForm)
xform = XForm.objects.get(kpi_asset_uid=self.asset.uid)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use the XForm object from the deployment, i.e.: self.asset.deployment.xform

self.xform.mongo_uuid
or f'{self.asset.owner.username}_{self.xform_id_string}'
)
except KeyError:
Copy link
Contributor

Choose a reason for hiding this comment

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

KeyError? when can this error happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review Olivier. The KeyError can occur when the formid key is missing from the backend_response dictionary. This is typically the case in certain test scenarios, such as ConflictingVersionsMockDataExports, where mock data doesn't include the formid key.

When self.xform is accessed, it attempts to retrieve the formid from backend_response, which raises a KeyError if it's not present.

Comment on lines 1246 to 1259
def generate_unique_key(self, length=20):

characters = string.ascii_letters + string.digits
# Generate a secure key with the specified length
return ''.join(secrets.choice(characters) for _ in range(length))

def set_mongo_uuid(self):
"""
Set the `mongo_uuid` for the associated XForm if it's not already set.
"""
if not self.xform.mongo_uuid:
self.xform.mongo_uuid = self.generate_unique_key()
self.xform.save(update_fields=['mongo_uuid'])

Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep methods sorted alphabetically and let's use KpiUidField.generate_unique_id() which already generates a random string (don't forget to clean imports after).

@rajpatel24 rajpatel24 requested a review from jnm as a code owner November 5, 2024 19:25
@rajpatel24 rajpatel24 removed the request for review from jnm November 5, 2024 19:26
@rajpatel24 rajpatel24 force-pushed the task-957-implement-mongo-owner-form-id branch from c85a06d to 98694f2 Compare November 5, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants