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

Added tags for images, events and news #103

Closed
wants to merge 2 commits into from
Closed

Conversation

OlleSettergren
Copy link
Contributor

Anything else that needs to be added at this stage?

This was linked to issues Dec 11, 2024
Copy link
Contributor

@Equasa Equasa left a comment

Choose a reason for hiding this comment

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

Generellt sätt en snygg pull request, dock krävs det mer arbete genom att bygga upp fler tables då detta skulle skapa galet mycket redundant utrymme i våran databas. Jag vill ha tables så som NewsTags osv istället


id: Mapped[int] = mapped_column(primary_key=True, init=False)
tag_name: Mapped[str] = mapped_column(String(MAX_TAG_NAME))
color: Mapped[str] = mapped_column(String(7)) # Defined as #ff0000 for example
Copy link
Contributor

Choose a reason for hiding this comment

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

tag_name: Mapped[str] = mapped_column(String(MAX_TAG_NAME))
color: Mapped[str] = mapped_column(String(7)) # Defined as #ff0000 for example

events: AssociationProxy[list["Event_DB"]] = association_proxy(
Copy link
Contributor

Choose a reason for hiding this comment

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

Detta är inte rätt implementerat, ska vara kopplat på annat sätt, skulle läsa att du vill göra typ en EventTags model också istället

target_collection="event_tags", attr="tag", init=False
)

news: AssociationProxy[list["News_DB"]] = association_proxy(target_collection="news_tags", attr="tag", init=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

samma sak som ovam


news: AssociationProxy[list["News_DB"]] = association_proxy(target_collection="news_tags", attr="tag", init=False)

images: AssociationProxy[list["Img_DB"]] = association_proxy(target_collection="image_tags", attr="tag", init=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

samma sak igen

@OlleSettergren
Copy link
Contributor Author

Generellt sätt en snygg pull request, dock krävs det mer arbete genom att bygga upp fler tables då detta skulle skapa galet mycket redundant utrymme i våran databas. Jag vill ha tables så som NewsTags osv istället

Hmm. Varje tag-modell kommer väl i princip bli ekvivalent med de andra förutom att man byter ut typen av lista? Finns det nåt smidigt sätt att göra det utan att behöva göra massa copy paste?

Som jag förstår det blir det samma problem sen när man väl ska definiera varje individuell tag, alltså att lägga in t.ex. "nollning" och "styrelsen" både som EventTag, ImageTag, NewsTag osv. Samma sak när man ska hämta allt innehåll kopplat till en viss tag.

@Equasa
Copy link
Contributor

Equasa commented Dec 22, 2024

Generellt sätt en snygg pull request, dock krävs det mer arbete genom att bygga upp fler tables då detta skulle skapa galet mycket redundant utrymme i våran databas. Jag vill ha tables så som NewsTags osv istället

Hmm. Varje tag-modell kommer väl i princip bli ekvivalent med de andra förutom att man byter ut typen av lista? Finns det nåt smidigt sätt att göra det utan att behöva göra massa copy paste?

Som jag förstår det blir det samma problem sen när man väl ska definiera varje individuell tag, alltså att lägga in t.ex. "nollning" och "styrelsen" både som EventTag, ImageTag, NewsTag osv. Samma sak när man ska hämta allt innehåll kopplat till en viss tag.

Asså grejen är att AssociationProxy löser inte alla problem åt oss utan man behöver skapa ett mellanliggande table...
Läs detta: https://chatgpt.com/share/6767e031-5698-8004-a2f0-a1408c7dafe8

@Equasa Equasa closed this Jan 19, 2025
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.

Add image tags Create Tags
2 participants