Skip to content

TA-3538 - Add tags table docs#67

Open
haakym wants to merge 7 commits intomasterfrom
TA-3538-tags
Open

TA-3538 - Add tags table docs#67
haakym wants to merge 7 commits intomasterfrom
TA-3538-tags

Conversation

@haakym
Copy link
Copy Markdown
Contributor

@haakym haakym commented Mar 24, 2026

Summary

TA-3538

Update documentation with new tags table

Changes

  • Create a new markdown file for the tags table with columns:
    • tag_guid
    • title
    • is_active
  • Add link to new page from nav
  • Update ER diagram

topics/tags.md Outdated
| ----------- | -------------- | ------------------------- | -------------------------------------- |
| `tag_guid` | `VARCHAR(64)` | The guid of the tag | `478A70D8-C627-91EC-D681-FFE617FABDD9` |
| `title` | `VARCHAR(max)` | The tag title | `Open access` |
| `is_active` | `boolean` | Enables or disables a tag | `0` or `1` |
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.

Comment: I think 0 and 1 are the correct values here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[question] Where did you get those values from? See here the values returned when querying are string values.

Copy link
Copy Markdown
Contributor Author

@haakym haakym Mar 26, 2026

Choose a reason for hiding this comment

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

Where did you get those values from?

A guess. Looking further into it I see there are multiple values that are acceptable based on the redshift docs on the Boolean type

State Valid literal values Storage
True TRUE 't' 'true' 'y' 'yes' '1' 1 byte
False FALSE 'f' 'false' 'n' 'no' '0' 1 byte
Unknown NULL 1 byte

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.

I'm going to make it true/false

@haakym haakym self-assigned this Mar 24, 2026
@haakym haakym added the 90% label Mar 24, 2026
@haakym haakym requested a review from Copilot March 24, 2026 16:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds documentation for the new f_rl_tags table and exposes it in the site navigation, along with a small ERD update to include the table in the relationships diagram.

Changes:

  • Add a new topic page documenting public.f_rl_tags and its columns.
  • Add “Tags” to the Application data navigation.
  • Extend the Mermaid ER diagram to include f_rl_tags.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
topics/tags.md New data dictionary page for the f_rl_tags table (columns + definitions).
topics/entity-relationships.md Adds f_rl_tags entity to the Mermaid ER diagram.
_includes/nav.html Adds a nav entry linking to the new Tags topic page.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

topics/tags.md Outdated
| Column Name | Datatype | Description | Example |
| ----------- | -------------- | ------------------------- | -------------------------------------- |
| `tag_guid` | `VARCHAR(64)` | The guid of the tag | `478A70D8-C627-91EC-D681-FFE617FABDD9` |
| `title` | `VARCHAR(max)` | The tag title | `Open access` |
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

VARCHAR(max) isn’t used anywhere else in the data dictionary and isn’t a valid type in Postgres/Redshift-style dialects (this repo consistently uses sized VARCHAR(n) like VARCHAR(255) / VARCHAR(4096)). Please align the title datatype with the actual column type and existing docs conventions (e.g. a sized VARCHAR(n) or TEXT as appropriate).

Suggested change
| `title` | `VARCHAR(max)` | The tag title | `Open access` |
| `title` | `VARCHAR(255)` | The tag title | `Open access` |

Copilot uses AI. Check for mistakes.
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.

Anyone else have thoughts on this. I based it on the value used in the PR to add the tags table:

s_rl_tags_: {
    columns: {
      tag_guid: 'varchar(64)',
      title: 'varchar(max)',
      is_active: 'boolean',

https://github.com/talis/census-server/pull/514/changes#diff-1b21636e546fea250308606e2500f45675c711aa8dc7cddd6e32bc60cf38bb64R1116

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@haakym haakym requested a review from a team March 24, 2026 16:38
@mikerudd mikerudd requested review from ksenijamv and mikerudd March 25, 2026 10:16
haakym and others added 3 commits March 26, 2026 16:55
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@ksenijamv ksenijamv left a comment

Choose a reason for hiding this comment

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

LGTM

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants