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.

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

Change candidate name and description from string to array of objects with required str, optional lang and dir fields #176

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

fsteeg
Copy link
Member

@fsteeg fsteeg commented Jul 10, 2024

With required `str`, optional `lang` and `dir` fields
And use for candidate `description`
Copy link

netlify bot commented Jul 10, 2024

Deploy Preview for reconciliation-api-specs ready!

Name Link
🔨 Latest commit 634ed9f
🔍 Latest deploy log https://app.netlify.com/sites/reconciliation-api-specs/deploys/67334460175641000822cc5b
😎 Deploy Preview https://deploy-preview-176--reconciliation-api-specs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@wetneb wetneb left a comment

Choose a reason for hiding this comment

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

Very nice!
I wonder if the same syntax should be adopted elsewhere, for instance in the responses for entity, name and type preview. I think it would be more consistent since those can be localized too, no?

wetneb

This comment was marked as duplicate.

@fsteeg
Copy link
Member Author

fsteeg commented Sep 11, 2024

I wonder if the same syntax should be adopted elsewhere, for instance in the responses for entity, name and type preview

You mean the suggest services (entity, property, type)? Additionally there are names and descriptions in data extension property proposals, names in data extension responses and a service name in the manifest. If we aim for a consistent data structure, should these all use the array-of-objects syntax?

At the same time, we support both simple strings and arrays of objects (or strings, even mixed) as condition values (v), see https://reconciliation-api.github.io/specs/draft/#example-7:

"conditions": [
    {
      "matchType": "name",
      "v": "Christel Hanewinckel"
    },
    {
      "matchType": "property",
      "pid": "professionOrOccupation",
      "v": [
        "Politik*",
        {
          "id": "wissenschaftler",
          "name": "Wissenschaftler(in)"
        }
      ]
    }
]

For consistency and maintainability we should probably unify these, right? To keep the common case simple, I'd prefer the current v behavior for all strings, but the discussion in #138 very much favors the consistent array-of-objects syntax (though it mostly mentions only the array part). But then v should also require consistent array-of-object values, right?

@wetneb
Copy link
Member

wetneb commented Sep 12, 2024

I think I have a preference for keeping the same format all the time ([{"str":"some value"}] in minimal form) even if it's a bit more verbose, because otherwise we're bound to have implementations which oversee this possibility and break when confronted with something else than a bare string.

- Extract existing string object definition to its own schema file
- Reference string-object.json in the suggest response schemas
- Update spec & examples to use string objects in suggest responses
- Redefine types used in suggest response as described in the spec
(instead of referencing the actual type.json schema)
- Clarify in the spec that we don't return actual full entity,
property, or type objects in the suggest response's `result` field
Also add `description` to spec and example (was in schema already)
@fsteeg
Copy link
Member Author

fsteeg commented Oct 9, 2024

I've updated suggest responses and property proposals to use the string object format too.

While there are fields all over the spec that in theory could also use the string object format, I think restricting it to the current state in this PR does actually make some sense: candidates, suggest responses and property proposals are all provided to users for making a selection. So this would allow multi-language objects for all user selections. At the same time, I keep wondering if this actually is the right approach. In his original requirement in #138, @saumier wrote:

[...] to return candidate names in one or more languages specified by the client request

Did we ever consider implementing it by (only) specifying the language(s) in the client request (in the Accept-Language header), and returning the old structure? Do we actually need to return multiple languages at the same time, for a single request?

I also noticed that in many places we don't use the actual object structure of our concepts (like entities, properties, types), but objects representing these concepts, but adding context-specific fields. I tried to clarify that a bit for the places I switched to the string object format.

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.

Support for multi-lingual candidate names
2 participants