[GSoC2024] Modified Attribute Serializer to retain values after request

[mach-12]

Motivation and context

Fixing #6502: While creating or editing a multiline text attribute on a label, it was replacing the default newlines with commas.

This behavior was not appropriate. The cause of this issue was identified as the DelimitedStringListField serializer:

cvat/cvat/apps/engine/serializers.py

Lines 226 to 240 in 70a7cc0

	class DelimitedStringListField(serializers.ListField):
	def to_representation(self, value):
	return super().to_representation(value.split('\n'))
	def to_internal_value(self, data):
	return '\n'.join(super().to_internal_value(data))
	class AttributeSerializer(serializers.ModelSerializer):
	values = DelimitedStringListField(allow_empty=True,
	child=serializers.CharField(allow_blank=True, max_length=200),
	)
	class Meta:
	model = models.AttributeSpec
	fields = ('id', 'name', 'mutable', 'input_type', 'default_value', 'values')

The fix was implemented by overriding the to_representation() method in the parent AttributeSerializer. This method converts the ListField into a list with only a single object with the text concatenated if the input_type in the context is 'text'.

Nested Label Serializers:

• LabelSerializer
  • SublabelSerializer
    • AttributeSerializer
      • DelimitedStringListField

How has this been tested?

Changes persist as intended in the Tasks, Projects and Job views.

Checklist

• I submit my changes into the develop branch
• I have created a changelog fragment
• I have updated the documentation accordingly
• I have added tests to cover my changes
• I have linked related issues (see GitHub docs)
• I have increased versions of npm packages if it is necessary
  (cvat-canvas,
  cvat-core,
  cvat-data and
  cvat-ui)

License

• I submit my code changes under the same MIT License that covers the project.
  Feel free to contact the maintainers if that's a concern.

[Marishka17]

Hello, thank you for the contribution.

I see one problem here:
In the current REST API implementation, it is possible to specify several values for text attributes too (even though it looks useless). But in that case, after these changes, all values will be merged into one and this is wrong.

We use one CharField to store values and \n to separate them. Probably you can change the separator (e.g. \\n), but in that case, Django migration must be added.

[mach-12]

it is possible to specify several values for text attributes too

Hey @Marishka17, I understand what you are trying to say here, But how does a user trigger this intended use case in the first place?

I understand you want User Input like this:

Hello,
This is a text<SEP>
Hello,
This is another text

To be stored like this:

values = ["Hello,\nThis is a text", "\nHello,\nThis is another text"]

Instead of this:

values = ["Hello,\nThis is a text\nHello,\nThis is another text"]

So I need to:

1. Define new separator,
2. Add appropriate db Migration

Question: How will the User know how to use it?

[Marishka17]

But how does a user trigger this intended use case in the first place

For example, users can use REST API directly

How will the User know how to use it?

A user can check the server endpoint specification.

So I need to:
Define new separator,
Add appropriate db Migration

It is possible to fix this issue using one more way - we can restrict specifying values for text attributes since this makes no sense at all. In that case, we don't need to change the separator.

IMHO, the second approach is more preferable.

[bsekachev]

Hi @mach-12

Do you have any plans regarding the pull request?

[mach-12]

Yes, I need some help with understanding how to restrict specification of text values. Going to finish this PR

[bsekachev]

@Marishka17, please, work together with @mach-12 to finish the PR.

[Marishka17]

Hello @mach-12, sorry for the delay, I'm going to close this PR now, but do not hesitate to reopen it when you implement more appropriate solution.

You can use the following approach to fix the issue:

• on the server: add AttributeSerialize.validate method and raise ValidationError if values is not empty and input_type == models.AttributeType.TEXT
• on the server: add migration to clean the values field for all text attributes
• on the client: exclude sending default value in attribute values field for text attributes