[GSoC2024] - fixed attribute rename issue

[ritikraj26]

Motivation and context

This PR fixes two open issues

• Fixed the issue - Unable to edit Attribute name Unble to edit Attribute name #5978
• Fixed the issue - REST API renaming an attribute does not work REST API renaming an attribute does not work #6503

Used attribute id to fetch attribute from the database and renamed it from the list received in the request.

How has this been tested?

Tested it under multiple scenarios, like different attribute types, input values, and labels.

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.

[codecov]

Codecov Report

Merging #7670 (9c83fa9) into develop (33f9cc2) will increase coverage by 0.00%.
Report is 1 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #7670   +/-   ##
========================================
  Coverage    83.44%   83.44%           
========================================
  Files          373      373           
  Lines        39724    39733    +9     
  Branches      3741     3741           
========================================
+ Hits         33147    33156    +9     
  Misses        6577     6577           

Components	Coverage Δ
cvat-ui	79.24% <ø> (ø)
cvat-server	87.32% <100.00%> (+<0.01%)	⬆️

[Marishka17]

Hello @ritikraj26, thank you for the contribution! We really appreciate your efforts. However, could you please consider implementing a slightly different approach?

Add a new id field to AttributeSerializer:

class AttributeSerializer(serializers.ModelSerializer):
    id = serializers.IntegerField(required=False)
    ...

In that case, you don't need to add explicitly id field into AttributeSpec model and create a new migration. Moreover, the attribute id will be available here (since each item of label.attributespec_set will include id) and you don't need to pass initial_data and "reinvent the wheel" on how to define attribute id.

Also, could you please add tests for checking that the renaming attributes functionality works as expected?

[ritikraj26]

Hello @ritikraj26, thank you for the contribution! We really appreciate your efforts. However, could you please consider implementing a slightly different approach?

Add a new id field to AttributeSerializer:

class AttributeSerializer(serializers.ModelSerializer):
    id = serializers.IntegerField(required=False)
    ...

In that case, you don't need to add explicitly id field into AttributeSpec model and create a new migration. Moreover, the attribute id will be available here (since each item of label.attributespec_set will include id) and you don't need to pass initial_data and "reinvent the wheel" on how to define attribute id.

Also, could you please add tests for checking that the renaming attributes functionality works as expected?

Sure I will add the tests for checking this functionality.

[ritikraj26]

Do I need to add the tests in test_labels.py or create a separate test_attributes.py?

[Marishka17]

Do I need to add the tests in test_labels.py or create a separate test_attributes.py?

Yes, there is no need to create a separate test file, you can extend TestPatchLabels from test_labels.py

[ritikraj26]

I will have to modify the payload here to adjust the expected output.
However making changes in the update_lablel is not affecting the response here
The response here is generated from the actual endpoints?
@Marishka17

[ritikraj26]

It seems to be working fine now. I had to clear the pytest cache.

[ritikraj26]

@Marishka17 @nmanovic
Updated the PR and added tests.

[Marishka17]

Probably it will be better to consider submitting a separate PR with REST API tests, to speed up the merging of this PR.

Also, could you please create a changelog fragment? (scriv create --edit)

[ritikraj26]

I removed the rest-api tests from this PR. I will add them in a separate PR.
@Marishka17 @nmanovic

[Marishka17]

@ritikraj26, I see that tests are failing now. Could you please update the server schema? (Run the command docker run --rm --entrypoint /bin/bash cvat/server:dev -c 'python manage.py spectacular' > cvat/schema.yml or use VS Code command server: Generate REST API Schema)

[ritikraj26]

@ritikraj26, I see that tests are failing now. Could you please update the server schema? (Run the command docker run --rm --entrypoint /bin/bash cvat/server:dev -c 'python manage.py spectacular' > cvat/schema.yml or use VS Code command server: Generate REST API Schema)

Done

[Marishka17]

@ritikraj26, I see that tests are failing now. Could you please update the server schema? (Run the command docker run --rm --entrypoint /bin/bash cvat/server:dev -c 'python manage.py spectacular' > cvat/schema.yml or use VS Code command server: Generate REST API Schema)

Done

@ritikraj26, Looks like you forgot to commit changes (there are still no changes in the cvat/schema.yml file).

[ritikraj26]

@ritikraj26, I see that tests are failing now. Could you please update the server schema? (Run the command docker run --rm --entrypoint /bin/bash cvat/server:dev -c 'python manage.py spectacular' > cvat/schema.yml or use VS Code command server: Generate REST API Schema)

Done

@ritikraj26, Looks like you forgot to commit changes (there are still no changes in the cvat/schema.yml file).

I pushed it. Could you check again?

[Marishka17]

LGTM

[Marishka17]

@ritikraj26, Thank you for the contribution!