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.

Sign up for GitHub

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

Jump to bottom

✨ [#4344] Configurable objects API group in ZGW registration #4758

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

✨ [#4344] Configurable objects API group in ZGW registration #4758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/openforms/js/components/admin/form_design/registrations/zgw/ZGWOptionsForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ const ZGWOptionsForm = ({index, name, label, schema, formData, onChange}) => {
const [modalOpen, setModalOpen] = useState(false);
const validationErrors = useContext(ValidationErrorContext);

const {zgwApiGroup, zaakVertrouwelijkheidaanduiding} = schema.properties;
const {zgwApiGroup, zaakVertrouwelijkheidaanduiding, objectsApiGroup} = schema.properties;
Copy link
Member

Choose a reason for hiding this comment

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

please update the component prop types accordingly

const apiGroupChoices = getChoicesFromSchema(zgwApiGroup.enum, zgwApiGroup.enumNames);
const objectsApiGroupChoices = getChoicesFromSchema(
objectsApiGroup.enum,
objectsApiGroup.enumNames
);
const confidentialityLevelChoices = getChoicesFromSchema(
zaakVertrouwelijkheidaanduiding.enum,
zaakVertrouwelijkheidaanduiding.enumNames
Expand Down Expand Up @@ -92,6 +96,7 @@ const ZGWOptionsForm = ({index, name, label, schema, formData, onChange}) => {
...formData,
// Ensure that if there's only one option, it is automatically selected.
zgwApiGroup: formData.zgwApiGroup ?? defaultGroup,
objectsApiGroup: formData.objectsApiGroup,
}}
onSubmit={(values, actions) => {
onChange({formData: values});
Expand All @@ -104,6 +109,7 @@ const ZGWOptionsForm = ({index, name, label, schema, formData, onChange}) => {
<ZGWFormFields
name={name}
apiGroupChoices={apiGroupChoices}
objectsApiGroupChoices={objectsApiGroupChoices}
confidentialityLevelChoices={confidentialityLevelChoices}
/>

Expand Down
29 changes: 28 additions & 1 deletion src/openforms/js/components/admin/form_design/registrations/zgw/ZGWOptionsFormFields.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
ValidationErrorContext,
ValidationErrorsProvider,
} from 'components/admin/forms/ValidationErrors';
import {ObjectsAPIGroup} from 'components/admin/forms/objects_api';

import BasicOptionsFieldset from './BasicOptionsFieldset';
import ManageVariableToPropertyMappings from './ManageVariableToPropertyMappings';
Expand All @@ -25,7 +26,21 @@ import {
} from './fields';
import {filterErrors} from './utils';

const ZGWFormFields = ({name, apiGroupChoices, confidentialityLevelChoices}) => {
/**
* Callback to invoke when the API group changes - used to reset the dependent fields.
*/
const onApiGroupChange = prevValues => ({
...prevValues,
objecttype: '',
objecttypeVersion: undefined,
});
sergei-maertens marked this conversation as resolved.
Show resolved Hide resolved

const ZGWFormFields = ({
name,
apiGroupChoices,
objectsApiGroupChoices,
confidentialityLevelChoices,
}) => {
const {
values: {propertyMappings = []},
} = useFormikContext();
Expand Down Expand Up @@ -112,6 +127,10 @@ const ZGWFormFields = ({name, apiGroupChoices, confidentialityLevelChoices}) =>
collapsible
fieldNames={['objecttype', 'objecttypeVersion', 'contentJson']}
>
<ObjectsAPIGroup
apiGroupChoices={objectsApiGroupChoices}
onApiGroupChange={onApiGroupChange}
/>
Comment on lines +130 to +133
Copy link
Member

Choose a reason for hiding this comment

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

I think this one should be isClearable to disable objects API integration again? Though clearing the objecttype field has the same effect

<ObjectType />
<ObjectTypeVersion />
<ContentJSON />
Expand All @@ -137,6 +156,14 @@ ZGWFormFields.propTypes = {
])
)
).isRequired,
objectsApiGroupChoices: PropTypes.arrayOf(
PropTypes.arrayOf(
PropTypes.oneOfType([
PropTypes.number, // value
PropTypes.string, // label
])
)
),
confidentialityLevelChoices: PropTypes.arrayOf(
PropTypes.arrayOf(PropTypes.string) // value & label are both string
).isRequired,
Expand Down
4 changes: 3 additions & 1 deletion ...enforms/js/components/admin/form_design/registrations/zgw/ZGWOptionsFormFields.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import {mockCaseTypesGet, mockCataloguesGet} from './mocks';

const NAME = 'form.registrationBackends.0.options';

const render = ({apiGroups, confidentialityLevelChoices, formData}) => (
const render = ({apiGroups, objectsApiGroupChoices, confidentialityLevelChoices, formData}) => (
<Formik initialValues={formData} onSubmit={fn()}>
<Form data-testid="test-form">
<ZGWFormFields
index={0}
name={NAME}
apiGroupChoices={apiGroups}
objectsApiGroupChoices={objectsApiGroupChoices}
confidentialityLevelChoices={confidentialityLevelChoices}
/>
</Form>
Expand All @@ -33,6 +34,7 @@ export default {
render,
args: {
apiGroups: [[1, 'ZGW API']],
objectsApiGroupChoices: [[1, 'Objects API']],
confidentialityLevelChoices: [
['openbaar', 'Openbaar'],
['geheim', 'Geheim'],
Expand Down
9 changes: 9 additions & 0 deletions src/openforms/registrations/contrib/zgw_apis/options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from zgw_consumers.api_models.constants import VertrouwelijkheidsAanduidingen

from openforms.api.fields import PrimaryKeyRelatedAsChoicesField
from openforms.contrib.objects_api.models import ObjectsAPIGroupConfig
from openforms.contrib.zgw.clients.catalogi import (
CaseType,
CatalogiClient,
Expand Down Expand Up @@ -104,6 +105,14 @@ class ZaakOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer):
)

# Objects API
objects_api_group = PrimaryKeyRelatedAsChoicesField(
queryset=ObjectsAPIGroupConfig.objects.exclude(
Q(objects_service=None) | Q(objecttypes_service=None)
),
help_text=_("Which Objects API set to use."),
label=_("Objects API set"),
required=False,
Copy link
Member

Choose a reason for hiding this comment

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

I think we may need an allow_null=True to make it explicit that no objects API integration is done.

There's some more steps involved in this PR I think:

  • A data migration that updates existing registration backends
    • if no objecttype is specified, ensure that objects_api_group is set to None
    • if an objecttype is specified, set the objects_api_group to the lowest PK (use that queryset that is now used at runtime)
    • this behaviour also needs to be applied during form import time when legacy forms are being imported (!), we can probably do something with the DEFINITION_CONVERTERS
  • Validation! Once we know which API group is used, we know the API root URl of the objecttypes API, and can check the URL provided. We can even look up the provided object type and check that it exists, but let's not do that yet because credentials to the object types API may not be configured yet. This should only come up when people edit the form again, and should not affect existing runtime behaviour of forms.

)
objecttype = serializers.URLField(
label=_("objects API - objecttype"),
help_text=_(
Expand Down
4 changes: 3 additions & 1 deletion src/openforms/registrations/contrib/zgw_apis/plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,9 @@ def register_submission_to_objects_api(
)

# In a follow up PR: the group will be configurable:
api_group = ObjectsAPIGroupConfig.objects.order_by("pk").first()
api_group = options.get(
"objects_api_group", ObjectsAPIGroupConfig.objects.order_by("pk").first()
)
Comment on lines +581 to +583
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to remove this default/fallback behaviour entirely so that all configuration is explicit, and we can then also define this accordingly in the options type declarations.

api_group = options["objects_api_group"]
assert api_group is not None

if not api_group: # pragma: no cover
raise RegistrationFailed("No API group available at all")
with get_objects_client(api_group) as objects_client:
Expand Down
6 changes: 6 additions & 0 deletions src/openforms/registrations/contrib/zgw_apis/tests/test_backend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1583,7 +1583,12 @@ def test_submission_with_zgw_and_objects_api_backends(self, m, obj_api_config):
),
)

# unused group
ObjectsAPIGroupConfigFactory.create(
objects_service__api_root="https://objecten2.nl/api/v1/",
organisatie_rsin="111222333",
)
objects_api_group = ObjectsAPIGroupConfigFactory.create(
objects_service__api_root="https://objecten.nl/api/v1/",
organisatie_rsin="000000000",
)
Expand Down Expand Up @@ -1723,6 +1728,7 @@ def get_create_json_for_zaakobject(req, ctx):
"organisatie_rsin": "000000000",
"zaak_vertrouwelijkheidaanduiding": "openbaar",
"doc_vertrouwelijkheidaanduiding": "openbaar",
"objects_api_group": objects_api_group,
"objecttype": "https://objecttypen.nl/api/v1/objecttypes/2",
"objecttype_version": 1,
}
Expand Down
3 changes: 3 additions & 0 deletions src/openforms/registrations/contrib/zgw_apis/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from typing import TYPE_CHECKING, Literal, NotRequired, TypedDict

if TYPE_CHECKING:
from openforms.contrib.objects_api.models import ObjectsAPIGroupConfig

from .models import ZGWApiGroupConfig


Expand Down Expand Up @@ -37,6 +39,7 @@ class RegistrationOptions(TypedDict):
organisatie_rsin: NotRequired[str]
zaak_vertrouwelijkheidaanduiding: NotRequired[VertrouwelijkheidAanduiding]
medewerker_roltype: NotRequired[str]
objects_api_group: NotRequired[ObjectsAPIGroupConfig]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
objects_api_group: NotRequired[ObjectsAPIGroupConfig]
objects_api_group: ObjectsAPIGroupConfig | None

I think we can enforce this key being set at all times. If the value is None, then no objects API registration is enabled (and we expect objecttype and objecttype_version to not be set at all, but either way they're ignored)

objecttype: NotRequired[str]
objecttype_version: NotRequired[int]
content_json: NotRequired[str]
Expand Down
Loading