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

feat: support favoriting topics #514

Merged
merged 1 commit into from
Dec 3, 2024
Merged

feat: support favoriting topics #514

merged 1 commit into from
Dec 3, 2024

Conversation

Jonas-C
Copy link
Contributor

@Jonas-C Jonas-C commented Nov 25, 2024

No description provided.

| "image"
| "audio"
| "video"
| "folder"
Copy link
Member

Choose a reason for hiding this comment

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

Folder kan fjernes fordi vi lagrer det på en anna måte.

Copy link
Member

Choose a reason for hiding this comment

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

Kan denne egentlig byttes ut med ResourceTypes fra myndla-api-typescript?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sikker? Får typefeil hvis jeg fjerner den

Copy link
Member

Choose a reason for hiding this comment

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

Med siste versjon også?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ja. Vi har eksplisitt støtte for henting av folders i samme fil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ser ut som at det har å gjøre med favorittmerking av mapper #448

Copy link
Member

Choose a reason for hiding this comment

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

Eg er klar over det men vi favorittmerker ikkje mapper. Vi lagrer dei. Dette blei aldri tatt i bruk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Da kan det fjernes. Foreslår at vi tar det i en ny PR isåfall!

Copy link
Member

Choose a reason for hiding this comment

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

Uenig. Bedre å slette det og bumpe backend-typer slik at dette blir korrekt.

Copy link
Contributor Author

@Jonas-C Jonas-C Dec 3, 2024

Choose a reason for hiding this comment

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

Enig i at det bør gjøres, men det har ikke noe med denne PR'en å gjøre. Ryddig å separere PR'ene våre til å kun handle om en ting om gangen. Kan ta inn #514 når denne er inne.

@@ -101,6 +112,9 @@ export const fetchFolderResourceMeta = async (
} else if (resource.resourceType === "multidisciplinary") {
const res = await fetchAndTransformResourceMeta([resource], context, "multidisciplinary");
return res[0] ?? null;
} else if (resource.resourceType === "topic") {
const res = await fetchTopicMeta([resource], context, "topic");
Copy link
Member

Choose a reason for hiding this comment

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

Kan ikkje denne bruke fetchAndTransformResourceMeta? Den brukes jo for multidisciplinary som er emner det også

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ja, man kan, men litt usikker på om jeg vil. fetchAndTransformResourceMeta bruker artikkel-ID, fordi det er artikkel som er i fokus og hentes. Node utledes fra artikkel-ID'en. fetchTopicMeta gjør det motsatte, og henter artikkel basert på noden man ber om. Separerte de egentlig for å vise at de har hvert sitt formål, men kan sikkert slå de sammen hvis du mener det er bedre!

Copy link
Member

Choose a reason for hiding this comment

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

Men vi lagrer vel artikkel-id på emnet også. Ikkje node-id så vil dette fungere i det heile tatt?

@Jonas-C Jonas-C force-pushed the feat/favorite-topics branch from c136408 to e862376 Compare December 3, 2024 09:01
@Jonas-C Jonas-C requested a review from gunnarvelle December 3, 2024 09:02
@Jonas-C Jonas-C merged commit 7a46c5c into master Dec 3, 2024
3 checks passed
@Jonas-C Jonas-C deleted the feat/favorite-topics branch December 3, 2024 17:50
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.

2 participants