Skip to content

Use field slugs in place of ids#116

Merged
3mcd merged 2 commits into
mainfrom
kalilsn/field-slugs
Sep 27, 2023
Merged

Use field slugs in place of ids#116
3mcd merged 2 commits into
mainfrom
kalilsn/field-slugs

Conversation

@kalilsn
Copy link
Copy Markdown
Contributor

@kalilsn kalilsn commented Sep 27, 2023

Issue(s) Resolved

Resolves #90

Test Plan

Basic smoke test of the integrations: reset your local database, then create a submission and an evaluation and make sure it works/fields are mapped properly

Additional context

I deleted the unused seed files partially just for cleanup, but also because pnpm build complains about type errors in those files even if they're not being imported and I don't want to spend time keeping them up to date when they're not being used. They'll be in the git history when we need them.

Comment thread core/lib/server/pub.ts
include: {
field: {
select: { name: true },
select: { slug: true },
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For now, I've just substituted slugs for names but kept the same "values" : { "<slug>": "<value>" } structure for pub responses. But I wonder if we should modify this structure so that we can include the name as well?

props.instanceId,
props.pub.id,
props.pub.values.Title as string,
props.pub.values['unjournal/title'] as string,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This doesn't really matter, and probably won't happen as much when we're not hardcoding field slugs into the integrations, but even so, should I change this <community>/<short-name> format so that we can still use . to access properties?

@isTravis isTravis temporarily deployed to kalilsn/field-slugs - core PR #116 September 27, 2023 00:16 — with Render Destroyed
@isTravis isTravis temporarily deployed to kalilsn/field-slugs - integration-evaluations PR #116 September 27, 2023 00:16 — with Render Destroyed
@isTravis isTravis temporarily deployed to kalilsn/field-slugs - integration-submissions PR #116 September 27, 2023 00:16 — with Render Destroyed
@kalilsn kalilsn requested a review from 3mcd September 27, 2023 00:18
Copy link
Copy Markdown
Collaborator

@3mcd 3mcd left a comment

Choose a reason for hiding this comment

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

LGTM!

@3mcd 3mcd merged commit 2235338 into main Sep 27, 2023
@3mcd 3mcd deleted the kalilsn/field-slugs branch September 27, 2023 16:42
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.

FieldID string slugs

3 participants