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

Add GeoJSON attachment params to EXPORT_OPTIONS #2757

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

Conversation

FrankApiyo
Copy link
Member

@FrankApiyo FrankApiyo commented Jan 8, 2025

Changes / Features implemented

  • Add new EXPORT_OPTIONS for GeoJSON exports

Steps taken to verify this change does what is intended

  • Tested locally
  • Updated test cases

Side effects of implementing this change

N/A

Before submitting this PR for review, please make sure you have:

  • Included tests
  • Updated documentation

Closes #2752

@FrankApiyo FrankApiyo changed the title Add gejson attachment params to EXPORT_OPTIONS [WIP]Add gejson attachment params to EXPORT_OPTIONS Jan 8, 2025
@FrankApiyo FrankApiyo changed the title [WIP]Add gejson attachment params to EXPORT_OPTIONS Add gejson attachment params to EXPORT_OPTIONS Jan 8, 2025
@FrankApiyo FrankApiyo force-pushed the fix-2752 branch 5 times, most recently from affcf0c to db854a3 Compare January 8, 2025 12:41
get_query_params_from_metadata(metadata),
)

metadata.delete()
Copy link
Contributor

Choose a reason for hiding this comment

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

@FrankApiyo Instead of deleting and creating a new MetaData object, we can update metadata.extra_data then do metadata.refresh_from_db()

@@ -349,6 +401,50 @@ def test_should_not_create_new_export_when_old_exists(self):
Export.objects.get(xform=self.xform).options,
)

# New metadata will yield a new export
metadata.delete()
Copy link
Contributor

Choose a reason for hiding this comment

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

@FrankApiyo Will updating metadata.extra_data suffice instead of deleting and creating a new one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whenever you create a new linked dataset, a new metadata object is created; So creating a new metadata object more closely simulates what happens in production. (as compared to editing an existing metadata object)

Copy link
Contributor

@kelvin-muchiri kelvin-muchiri Jan 9, 2025

Choose a reason for hiding this comment

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

@FrankApiyo Noted. Though l believe since this is a unit test and not an integration test, we should only be interested in the metadata passed to custom_response_handler and not the larger context of how the MetaData object is used

@ukanga ukanga changed the title Add gejson attachment params to EXPORT_OPTIONS Add GeoJSON attachment params to EXPORT_OPTIONS Jan 9, 2025
},
object_id=self.xform.id,
)
_response = custom_response_handler(
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the variable _response is unused

Copy link
Member Author

Choose a reason for hiding this comment

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

This _response variables might provide some implicit documentation by telling you that this custom_reponse_handler returns a response object

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GeoJSON export not re-generated when properties change
2 participants