Skip to content

Conversation

@Floris272
Copy link
Contributor

@Floris272 Floris272 commented Dec 4, 2025

Partially fixes #564

Changes

  • adds ObjectType fields to existing ObjectType model
  • adds ObjectTypeVersionModel
  • adds command to import types and versions from an objecttypes api service

To make the migration easier i would suggest to first only merge this pr that adds the model and the command and add everything from objecttypes in the next pr/release.

The update path would be to update objects to 3.6.0 which will include the script which just needs to be run before updating to 4.0.0 which would include the whole refactor.

@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 92.77108% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.62%. Comparing base (6272c8c) to head (1305ff0).
⚠️ Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
src/objects/core/models.py 89.79% 4 Missing and 1 partial ⚠️
src/objects/core/utils.py 50.00% 5 Missing ⚠️
...cts/core/management/commands/import_objecttypes.py 96.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #711      +/-   ##
==========================================
+ Coverage   84.12%   84.62%   +0.50%     
==========================================
  Files         134      138       +4     
  Lines        2576     2738     +162     
  Branches      208      215       +7     
==========================================
+ Hits         2167     2317     +150     
- Misses        362      373      +11     
- Partials       47       48       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

🐰 Bencher Report

Branchfeature/564-import-objecttypes-command
Testbedubuntu-latest

🚨 1 Alert

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Upper Boundary
(Limit %)
performance_test/tests/test_objects_list.py::test_objects_api_list_filter_one_resultLatency
milliseconds (ms)
📈 plot
🚷 threshold
🚨 alert (🔔)
22.48 ms
(+6.26%)Baseline: 21.16 ms
22.21 ms
(101.20%)

Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
performance_test/tests/test_objects_list.py::test_objects_api_list_filter_by_object_type📈 view plot
🚷 view threshold
118.53 ms
(+3.25%)Baseline: 114.80 ms
120.54 ms
(98.33%)
performance_test/tests/test_objects_list.py::test_objects_api_list_filter_one_result📈 view plot
🚷 view threshold
🚨 view alert (🔔)
22.48 ms
(+6.26%)Baseline: 21.16 ms
22.21 ms
(101.20%)

performance_test/tests/test_objects_list.py::test_objects_api_list_large_page_size_page_1📈 view plot
🚷 view threshold
276.13 ms
(+1.52%)Baseline: 272.00 ms
285.60 ms
(96.68%)
performance_test/tests/test_objects_list.py::test_objects_api_list_large_page_size_page_5📈 view plot
🚷 view threshold
278.99 ms
(+1.59%)Baseline: 274.61 ms
288.34 ms
(96.76%)
performance_test/tests/test_objects_list.py::test_objects_api_list_small_page_size_page_20📈 view plot
🚷 view threshold
132.08 ms
(+2.93%)Baseline: 128.32 ms
134.74 ms
(98.03%)
🐰 View full continuous benchmarking report in Bencher

@Floris272 Floris272 marked this pull request as ready for review December 5, 2025 15:59
@Floris272 Floris272 requested a review from stevenbal December 5, 2025 15:59
@stevenbal
Copy link
Collaborator

@alextreme is this what you had in mind in terms of releases for combining Objects+Objecttypes? I guess instead of 3.7.0 it would be a 4.0.0 release? Or do we want to release 3.7.0 where we both support the old situation + local objecttypes?

To make the migration easier i would suggest to first only merge this pr that adds the model and the command and add everything from #564 (comment) in the next pr/release.

The update path would be to update objects to 3.6.0 which will include the script which just needs to be run before updating to 3.7.0 which would include the whole refactor.

@alextreme
Copy link
Member

@alextreme is this what you had in mind in terms of releases for combining Objects+Objecttypes? I guess instead of 3.7.0 it would be a 4.0.0 release? Or do we want to release 3.7.0 where we both support the old situation + local objecttypes?

I would indeed make this a major version, in which we drop the old situation (external Objecttypes).

How this should work for 3.7.0 is up for debate. Some options:

  1. Support only the old situation, the import can be performed but local objecttypes after the import aren't used
  2. Support the old situation and any local objecttypes (however what happens after doing the import, won't this conflict?)
  3. Support the old situation until the import command is run (eg. if there are local objecttypes then disable the old situation)

I don't have a preference, up to you @stevenbal to make the call

@stevenbal
Copy link
Collaborator

I'm inclined to go for option 1, that minimizes the amount of code paths and it also gives an incentive for people to upgrade to 4.0

@alextreme
Copy link
Member

  1. is the same approach as what OF did with Ogone->Worldline and this worked well. Agreed as long as we ensure it's properly mentioned in the changelog

@Floris272 Floris272 requested a review from stevenbal December 16, 2025 12:03
@stevenbal
Copy link
Collaborator

@alextreme should this only be a management command, or should it also be possible to run from the admin? Since I remember the discussion about devops not wanting to run commands?

@stevenbal
Copy link
Collaborator

@Floris272 I also noticed src/manage.py import_objecttypes --help raises errors:

TypeError: expected string or bytes-like object, got 'tuple'

@Floris272 Floris272 requested a review from stevenbal December 16, 2025 15:11
@alextreme
Copy link
Member

@alextreme should this only be a management command, or should it also be possible to run from the admin? Since I remember the discussion about devops not wanting to run commands?

I'd say give the option to do it either way, this django app ( https://github.com/Lcrs123/django-admin-commander ) looks interesting but the approach is up to you

Copy link
Collaborator

@stevenbal stevenbal left a comment

Choose a reason for hiding this comment

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

@Floris272 added a checklist item in the issue for running the command via the admin and marked it as nice to have, if we have time left we can implement that

@Floris272 Floris272 merged commit 47cde92 into master Dec 18, 2025
27 checks passed
@Floris272 Floris272 deleted the feature/564-import-objecttypes-command branch December 18, 2025 14:45
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.

OpenObjecten - combine the ObjectsType API into the Objects API

5 participants