Skip to content
This repository has been archived by the owner on Mar 15, 2018. It is now read-only.

redirections #10

Merged
merged 13 commits into from
Jan 7, 2018
Merged

redirections #10

merged 13 commits into from
Jan 7, 2018

Conversation

patte
Copy link
Contributor

@patte patte commented Jan 4, 2018

affects: @orbiting/backend-modules-redirections

TODOs:

  • slug change -> redirection -> slug change back to original -> only keep second change back to original

affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
@patte
Copy link
Contributor Author

patte commented Jan 4, 2018

@tpreusse I'm against naming the graphql query / type Resource. You get back a source and target, so it's a redirection between resources or a pointer to a resource but no a resource as such. Maybe I get your point when I can read it.
According return-nothing vs. return always with status 404 I'm open.

@tpreusse
Copy link
Contributor

tpreusse commented Jan 4, 2018

@patte I agree that it doesn't make sense—here—in a redirection module. But it would make sense in republik-backend: (find)resource(path: String) would give the backend the power to get its hand on any unmatched path, try to find the entity and respond with the correct status (503—upstream backend down, 410—get ride of it forever, 404—not found, redirects etc.). It can only be the backend which is aware of everything not some module that only know where little about the world. I'd recommend to only collect the lib utils for redirection in this module and do the graph ql endpoint as resource in republik-backend. But we can also leave it as is and do a find resource later.

@patte
Copy link
Contributor Author

patte commented Jan 4, 2018

Alright I get it. So let's keep this as is, redirections for documents and users, then add a (find)resource query to republik-backend on top of it for awesome redirection/resolve magic. Because we have many other tasks right now, I'd prefer to postpone that. You agree?

"id" uuid primary key not null default uuid_generate_v4(),
"source" text not null,
"target" text not null,
"status" integer not null default 308,
Copy link
Contributor

Choose a reason for hiding this comment

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

should be 301

Copy link
Contributor

Choose a reason for hiding this comment

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

only other regular value for redirect is 302

Copy link
Contributor Author

Choose a reason for hiding this comment

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

308, strange ;)

}

type queries {
# empty response: 404
Copy link
Contributor

Choose a reason for hiding this comment

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

null is fine for redirect and easier right? resource needs to be 404 ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

rm docu here—it is returning null and not returning a 404

{
"source": "/me",
"target": "/~me",
"status": 308
Copy link
Contributor

Choose a reason for hiding this comment

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

301

affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
…eletedAt in get

affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
@patte patte changed the title wip: redirections redirections Jan 7, 2018
"createdAt" timestamptz default now(),
"updatedAt" timestamptz default now(),
"deletedAt" timestamptz,
UNIQUE("source")
Copy link
Contributor

Choose a reason for hiding this comment

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

not unique with deletedAt

}

type queries {
# empty response: 404
Copy link
Contributor

Choose a reason for hiding this comment

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

rm docu here—it is returning null and not returning a 404

redirections
WHERE
source = :source AND
NOT (resource @> :resource) AND
Copy link
Contributor

Choose a reason for hiding this comment

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

is undefiend handled?

affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
affects: @orbiting/backend-modules-redirections
@patte patte merged commit 8aef262 into master Jan 7, 2018
@patte patte deleted the redirections branch January 7, 2018 16:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants