Locations : Set error messages similar to targets/enumerators end point

[utkarsh14]

Locations : Set error messages similar to targets/enumerators end point

Ticket

Fixes: UX fixes

Description, Motivation and Context

Changed locations endpoint to use record_errors and summary_by_error_type in error messages, this links to frontend changes where we are adding an error warning table for locations.

How Has This Been Tested?

Local

Checklist:

This checklist is a useful reminder of small things that can easily be forgotten.
Put an x in all the items that apply and remove any items that are not relevant to this PR.

• My code follows the style guidelines and standard practices for this project
• I have reviewed my own code to ensure good quality
• I have tested the functionality of my code to ensure it works as intended
• I have resolved merge conflicts
• I have written good commit messages
• I have created migration scripts from the latest dev changes. This will prevent migration script version conflicts (if applicable)
• I have scrutinized and edited the migration script with reference to changes Alembic won't auto-detect (if applicable). Note that this includes manually adding the creation of new CHECK constraints.
• I have updated the automated tests (if applicable)
• I have updated the README file (if applicable)
• I have updated the OpenAPI documentation (if applicable)

[Copilot]

Pull Request Overview

This PR refactors how nullability and bulk-editability are represented in the targets module and standardizes error reporting in the locations endpoints by adopting a record_errors structure with summary_by_error_type.

• Replaces the bulk_editable column/flag with allow_null_values across migrations, model, controllers, tests, and API docs.
• Updates location file validations and error handlers to emit a record_errors object containing a summary_by_error_type list of detailed error entries.
• Adjusts unit tests and OpenAPI documentation to align with the new error response and configuration schema.

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/test_targets.py	Updated tests to assert allow_null_values instead of bulk_editable.
tests/unit/test_locations.py	Modified tests to verify the new record_errors.summary_by_error_type structure.
migrations/versions/b0c4574bd52e_.py	Added allow_null_values column and removed bulk_editable in migration.
app/blueprints/targets/models.py	Replaced bulk_editable column with allow_null_values.
app/blueprints/targets/controllers.py	Swapped references to bulk_editable for allow_null_values; adjusted bulk-update logic.
app/blueprints/locations/utils.py	Refactored validation to accumulate record_errors.summary_by_error_type entries.
app/blueprints/locations/controllers.py	Transitioned error handlers to produce the new record_errors format.
app/blueprints/docs/surveystream.yml	Updated OpenAPI spec to use allow_null_values and reflect new response schema.

Comments suppressed due to low confidence (1)

app/blueprints/targets/controllers.py:1013

• Removing the check on bulk_editable means all columns except target_id are now treated as bulk-editable. Verify that this aligns with intended behavior or reintroduce a dedicated flag to control which fields can be bulk updated.

        if column.column_name != "target_id":

[Copilot]

The comprehension is applied to the outer dict rather than inside the summary_by_error_type list. It should be a list comprehension within the list literal to avoid overwriting the same key on each iteration.

Suggested change

	]
	for error_message in e.geo_level_hierarchy_errors
	for error_message in e.geo_level_hierarchy_errors
	],

[Copilot]

[nitpick] Consider using server_default=sa.text('true') instead of a raw string to ensure the default boolean is emitted correctly in the SQL DDL.

Suggested change

	"allow_null_values", sa.Boolean(), server_default="true", nullable=False
	"allow_null_values", sa.Boolean(), server_default=sa.text('true'), nullable=False

[Copilot]

[nitpick] Catching a bare Exception can mask unexpected issues and make debugging harder. Consider catching more specific exceptions or logging before rethrowing.

Suggested change

	except Exception as e:
	db.session.rollback()
	return jsonify(message=str(e)), 500
	except ValueError as e: # Example of a specific exception
	db.session.rollback()
	return jsonify(message=f"Value error occurred: {str(e)}"), 400
	except Exception as e:
	db.session.rollback()
	# Log the unexpected exception for debugging purposes
	app.logger.error(f"Unexpected error: {str(e)}", exc_info=True)
	raise

[jeenut27]

Left one comment. Rest looks good! Thanks

[jeenut27]

@utkarsh14 - if you compare the error message here vs that of targets you will see targets has extra invalid_records and summary keys. I believe invalid_records is needed for the download output which was discussed. And summary is used for summary stats on UI. ?

[utkarsh14]

Done

[jeenut27]

Changes look good. Thanks!

[sentry]

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

• ‼️ InvalidIndexError: Reindexing only valid with uniq... in production-callisto

_{Did you find this useful? React with a 👍 or 👎}