Skip to content

Consider renaming database and/or API fields for clarity #555

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

Open
carrythebanner opened this issue Jan 15, 2024 · 3 comments
Open

Consider renaming database and/or API fields for clarity #555

carrythebanner opened this issue Jan 15, 2024 · 3 comments

Comments

@carrythebanner
Copy link
Collaborator

carrythebanner commented Jan 15, 2024

Some fields in the calevent and caldaily database tables can be a bit difficult to understand. Some examples:

  • calevent.name is the organizer name, calevent.title is the event title; "name" can be misconstrued as "event name"
  • calevent.locdetails instead of location_details
  • calevent.time is implicitly start time, but calevent.endtime is explicitly end time
  • caldaily.id is actually a foreign key pointing to calevent.id, and caldaily.pkid is the "true" id of the occurrence

MySQL allows columns to be up to 64 characters long, so we have some room if we want to make them longer for clarity.

Related, the events API output mainly mirrors the database column names, but doesn't strictly need to.

  • The caldaily_id field surfaces the internal db name which is irrelevant to API consumers. We could probably make it something more meaningful like occurrence_id.
  • Some fields aren't useful, e.g. hideemail. If hideemail = true then the backend ensures that email = null in the output; there shouldn't be anything for the API consumer to do. If hideemail = false then the email is provided and there's also nothing for the API consumer to do.
  • Boolean fields like featured could possibly be renamed to is_featured to better match boolean conventions.

This is also generally true for the manage_event and retrieve_event endpoints, though we don't currently (and haven't historically) had any external consumers for those APIs. We should keep consistency between the endpoints where possible and beneficial, though.

@carrythebanner
Copy link
Collaborator Author

Regarding the "hide" fields, the front-end is currently using those flags (example) but we could just check on the presence of a value instead (e.g. check email instead of hideemail). Also, if the flag is false (don't hide) but a value isn't set, there's nothing the client can do to show a value.

Also the preview mode of the edit form does have a legit purpose for those. See this example PR. We might be able to read directly from the current form state instead of relying on the API, though. See previewEvent() in addevent.js; might be able to check hide flags from the form, and then null the associated field values just for the preview representation. If we make the above changes, then this would actually be a more accurate way to preview as it would mimic the regular front-end display more closely.

@carrythebanner
Copy link
Collaborator Author

Renaming fields would start to break compatibility with code in the legacy folder. We're not running any of that now, but if we wanted to revive any of that we'd need to do extra work to bring it inline.

@carrythebanner
Copy link
Collaborator Author

Another one: "hidden" vs. "published." The database uses hidden but "negative term = true" can be a bit confusing. Largely we use the phrase "publish" for user-facing bits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant