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.

Sign up for GitHub

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

Jump to bottom

Setup fsxa #6

Merged
merged 14 commits into from
Jan 26, 2023
Merged

Setup fsxa #6

merged 14 commits into from
Jan 26, 2023

Conversation

lksmsr
Copy link
Contributor

@lksmsr lksmsr commented Jan 25, 2023

No description provided.

Code42Cate
Code42Cate reviewed Jan 26, 2023
View reviewed changes
types.ts Outdated
pathToClientAccessControlConfig?: string; // EXPERIMENTAL optional path to file that exports client access conrtol
}

export interface FSXAFileConfig extends Partial<FSXAConfig> {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wird nicht genutzt, oder?

Suggested change
export interface FSXAFileConfig extends Partial<FSXAConfig> {}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Je nachdem was mit FSXAFileConfig passiert würde ich FSXAConfig dort definieren, wo es relevant ist und genutzt wird, nämlich die eigentliche config in app.config.ts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmn. sollte die fsxa-api noch einen type exportieren, der Fileconfig heißt, deswegen hab ichs in dem Types ordner gelassen. Das ist ja nichts, worin der Endkunde drin rumfummeln sollte. Und in app.config.ts sollte nur configuration stehen.

package.json Show resolved Hide resolved
plugins/2.setupProxyApi.ts Outdated
const runtimeConfig = useRuntimeConfig();
const appConfig = useAppConfig();
const fsxaApi = new FSXAProxyApi(
`${runtimeConfig.public.baseURL}/api`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
`${runtimeConfig.public.baseURL}/api`,
new URL('/api', runtimeConfig.public.baseURL).href,

String concat ist hier bissl gefährlich, weil wir absolut keine Ahnung haben ob baseURL das richtige Format hat und wir den Fehler erst während Runtime sehen, wenn unsere API Requests failen. Würde zumindest mal die urls "richtig" kombinieren um sowas wie localhost:3000//api zu verhindern.

server/api/[endpoint].post.ts
import { ServerErrors, FSXAProxyRoutes, FSXAApiErrors } from "~/types";

export default defineEventHandler(async (event) => {
const remoteApi = FSXAApiSingleton.instance; // throws error if undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also gibts einen 500er wenn FSXAApiSingleton.instance undefiniert ist? Kann das tatsächlich passieren?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vermutlich höchst unwahrscheinlich, aber ändert ja nichts an dem Verhalten... das da ein Error geworfen wird kommt aus dem fsxa-api repo

server/api/[endpoint].post.ts
Comment on lines +23 to +30
err.message === FSXAApiErrors.NOT_FOUND ||
err.message === FSXAApiErrors.UNKNOWN_REMOTE
) {
throw createError({
statusCode: 404,
message: err.message,
});
} else if (FSXAApiErrors.NOT_AUTHORIZED === err.message) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sehr unangenehm, dass wir hier anhand der Error Message erkennen müssen was schief gegangen ist. Sollte eigentlich nicht nötig sein wenn die api das gut implementieren würde :(

Um das ganze trotzdem bisschen type safe zu machen würde ich hier err als unknown behalten und mit instanceof schauen ob wir wirklich auf message zugreifen können bevor wir irgendwas machen (Da wir hier eine promise catchen könnte der error tatsächlich alles sein). Und sonst wahrscheinlich auch einfach 500 schmeißen weil wir nichts mit dem error anfangen können.

.env.template
@@ -1,3 +1,4 @@
BASE_URL=http://localhost:3000
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wir benutzen BASE_URL relativ häufig, aber Nuxt ignoriert das komplett (vor allem der port ist relevant). Können wir das noch in der Nuxt config integrieren, damit wir da die BASE_URL nicht nochmal hardcoden müssen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weiß ich nicht genau wie du das meinst... Base url steht in der nuxt config, in der Template ist es hier als optionaler parameter gedacht

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aber wenn du nen gute verbesserung hast, feel free

@Code42Cate Code42Cate merged commit 2f10a0b into main Jan 26, 2023
@Code42Cate Code42Cate deleted the setup-fsxa branch January 26, 2023 17:37
@lksmsr
Copy link
Contributor Author

lksmsr commented Jan 26, 2023

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@lksmsr
Copy link
Contributor Author

lksmsr commented Feb 13, 2023

🎉 This PR is included in version 1.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@lksmsr
Copy link
Contributor Author

lksmsr commented Feb 13, 2023

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

dp-technical pushed a commit that referenced this pull request Aug 17, 2023
@semantic-release-bot
chore(release): 1.0.0 [skip ci]
06a6e35 
# 1.0.0 (2023-08-17)

### Bug Fixes

* **baseURL-config:** remove baseURL configuration ([#106](#106)) ([13ffc51](13ffc51))
* **button:** fix internal links ([#39](#39)) ([c1b210d](c1b210d))
* **components:** hide dev ([#44](#44)) ([ac3e759](ac3e759))
* **components:** hydration mismatch warnings ([#19](#19)) ([7a8d60c](7a8d60c))
* **dependencies:** update fsxa dependencies ([4a0b404](4a0b404))
* **dependencies:** update fsxa dependencies ([#88](#88)) ([c7b02ae](c7b02ae))
* **dependencies:** update to latest content-api version ([#100](#100)) ([230085e](230085e))
* **devcomponents:** hide if not preview mode ([#59](#59)) ([38e399d](38e399d))
* **dev:** fix dev component ([#53](#53)) ([ba74bb2](ba74bb2))
* **error.vue:**  exclamation mark style issue ([#64](#64)) ([cb22090](cb22090))
* **footer component:** legal links in footer now react to language switch ([de9fe56](de9fe56))
* **footer:** fix links on footer ([563c1ba](563c1ba))
* **languageswitch:** resolve language switch mismatch ([#86](#86)) ([511bb3c](511bb3c))
* **package.json:** repository url config semantic release ([7fae7a9](7fae7a9))
* **post-endpoint:** remove attribute mapping from request body ([10a0cbe](10a0cbe))
* **richtext:** add support for nested texts ([#20](#20)) ([e2b4c42](e2b4c42))
* **richtextelement:** render content as html ([#37](#37)) ([6373736](6373736))
* **routing:** correctly handle content projection ([#32](#32)) ([8117380](8117380))
* **routing:** differentiate between 404 and 500 ([#76](#76)) ([d16927c](d16927c))
* **routing:** redirect to new route after locale changed ([#22](#22)) ([20e398a](20e398a))
* **slug:** use showError instead of createError in useAsyncData ([#35](#35)) ([588e021](588e021))
* style improvements ([#50](#50)) ([4c85903](4c85903))
* todos in code ([#73](#73)) ([ac0d188](ac0d188))

### Features

* **components:** product category section ([#36](#36)) ([7f5114c](7f5114c))
* **components:** setup component wireframes ([#8](#8)) ([8e882c2](8e882c2))
* **content:** add caching layer to content ([#18](#18)) ([9c0e50f](9c0e50f))
* **content:** fetch and store dataset in content cache ([#33](#33)) ([72cf484](72cf484))
* **dev:** add syntax highlighting ([#69](#69)) ([97f516d](97f516d))
* **dev:** display products data ([#40](#40)) ([729b67a](729b67a))
* **endpoint:** create an endpoint to check for the health of server ([1430d75](1430d75))
* **featuredProducts:** add component ([2c2f331](2c2f331))
* **featuredproductsection:** add 2 new components: FeaturedProducts and FeaturedProductItem ([bdaa19c](bdaa19c))
* **locale:** change default lang via env ([#67](#67)) ([454ca7b](454ca7b))
* **locales:** get available locales ([#83](#83)) ([d36ad50](d36ad50))
* **logger:** add logger plugin ([#56](#56)) ([b569b30](b569b30))
* **logger:** import Logger class from fsxa-api ([a4a0739](a4a0739))
* **logger:** import Logger class from fsxa-api ([61f71ef](61f71ef))
* **product:** style product section ([#34](#34)) ([96e23aa](96e23aa))
* **projectProperties:** fetch pp on app start ([#15](#15)) ([a61f151](a61f151))
* **richtext:** add u element component ([f20dcb1](f20dcb1))
* setup basic navigation ([#7](#7)) ([97c9cd5](97c9cd5))
* setup fsxa ([#6](#6)) ([2f10a0b](2f10a0b))
* **sitemap:** add sitemap ([#47](#47)) ([5b18af6](5b18af6))
* style layout components ([#25](#25)) ([a379a98](a379a98))
* **tpp:** add editing functionality ([#51](#51)) ([69a8159](69a8159))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Code42Cate Code42Cate Code42Cate left review comments

Assignees
No one assigned
Labels
released on @alpha released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lksmsr @Code42Cate