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

Remove url prefixing on frontend #1054

Closed
pavish opened this issue Feb 8, 2022 · 7 comments · Fixed by #1171
Closed

Remove url prefixing on frontend #1054

pavish opened this issue Feb 8, 2022 · 7 comments · Fixed by #1171
Assignees
Labels
affects: dx Related to developer experience affects: technical debt Improves the state of the codebase good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this type: enhancement New feature or request work: frontend Related to frontend code in the mathesar_ui directory

Comments

@pavish
Copy link
Member

pavish commented Feb 8, 2022

Description

Currently, the frontend appends a url prefix /api/v0 in the api util. This was implemented earlier for two reasons:

  • Inorder to easily update the api version for future releases, considering the client updates it all with each release.
  • Inorder to avoid explicitly adding this to everywhere we need to make a request.

However, with the changes done for #1041, both these reasons are now void:

  • We now have two API namespaces, each maintaining their own versions.
  • We may have to work with multiple API versions of the same namespace on the client in the future.

We need to remove the common method appendUrlPrefix within /mathesar_ui/src/utils/api.ts and explicitly mention the prefix in each place where a request is made.

@pavish pavish added type: enhancement New feature or request good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this affects: dx Related to developer experience affects: technical debt Improves the state of the codebase work: frontend Related to frontend code in the mathesar_ui directory ready Ready for implementation labels Feb 8, 2022
@pavish pavish added this to the [10.1] 2022-03 improvements milestone Feb 8, 2022
@sanjeevbhusal
Copy link

Hey, is anyone assigned to it?

@kgodey
Copy link
Contributor

kgodey commented Feb 9, 2022

@sanjeevbhusal Not yet, do you want to work on it?

@nimishbongale
Copy link
Contributor

nimishbongale commented Mar 13, 2022

Hello @kgodey, can I work on this issue?

And just a side question, do we want this prefix URL to be configurable?

@kgodey kgodey added status: started and removed ready Ready for implementation labels Mar 13, 2022
@kgodey
Copy link
Contributor

kgodey commented Mar 13, 2022

Sure, go ahead @nimishbongale. I'll defer to @pavish on whether it should be configurable.

@pavish
Copy link
Member Author

pavish commented Mar 14, 2022

@nimishbongale

do we want this prefix URL to be configurable

No, we do not need it to be configurable at the moment.

@pavish
Copy link
Member Author

pavish commented Mar 15, 2022

A bit more information on this issue:

Whenever we make a request from the frontend (In places where we call methods like postAPI, getAPI etc.,), the appendUrlPrefix method checks if the full path is specified, if yes, it does not append any prefix, otherwise it appends the db prefix by default.

As mentioned in the issue description above, this is no longer desired since we now deal with multiple namespaces and possibly the need to use multiple versions of the same namespace.

We need to specify the full url everywhere requests are made.
eg., In stores/schemas.ts,

const response = await postAPI<SchemaResponse>('/schemas/', {
    name: schemaName,
    database,
})

will change to:

const response = await postAPI<SchemaResponse>('/api/db/v0/schemas/', {
    name: schemaName,
    database,
})

@nimishbongale
Copy link
Contributor

A bit more information on this issue:

Whenever we make a request from the frontend (In places where we call methods like postAPI, getAPI etc.,), the appendUrlPrefix method checks if the full path is specified, if yes, it does not append any prefix, otherwise it appends the db prefix by default.

As mentioned in the issue description above, this is no longer desired since we now deal with multiple namespaces and possibly the need to use multiple versions of the same namespace.

We need to specify the full url everywhere requests are made. eg., In stores/schemas.ts,

const response = await postAPI<SchemaResponse>('/schemas/', {
    name: schemaName,
    database,
})

will change to:

const response = await postAPI<SchemaResponse>('/api/db/v0/schemas/', {
    name: schemaName,
    database,
})

I think I have a clearer understanding now, will make the necessary changes! Thanks @pavish!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: dx Related to developer experience affects: technical debt Improves the state of the codebase good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. help wanted Community contributors can implement this type: enhancement New feature or request work: frontend Related to frontend code in the mathesar_ui directory
Projects
No open projects
4 participants