Perform a test through the URL directly

ghost commented 2 years ago

Purpose

It has been discussed to be able to easily pass a domain to test to Zonemaster GUI, without interacting with the form. The idea is to pass the domain name in the URL /check/<domain>. The form's input field will be populated with this domain name and the form will be sent.

No option are currently passed. Only the domain. As I see it, we could extend this work to include options as GET parameters.

Context

Addresses #200 and follow-up on #335

Changes

New route /check/<domain>
Redirect /domain_check to /check
Redirect /test/<test-id> to /result/<test-id>

How to test this PR

access /check/<domain> and make sure the test is started for \<domain>
once on the result page, go back to the previous page, the form should be sent again and the results page should be loaded
e2e tests have been added to test both redirections

matsduf commented 2 years ago

Would it be relevant to be able to include the language setting? /domain_check/lang=<lang>/<domain> or /domain_check/<lang>/<domain>. The former is more flexible and could be opened for eg. /ns=<ns> in the future

matsduf commented 2 years ago

I also want to come back to comment in the issue (https://github.com/zonemaster/zonemaster-gui/issues/200#issuecomment-825571535): "This is a good idea. The first step is the design and specification of the URL schema. It should be documented as an API to make sure that we do not change anything without an explicit decision."

ghost commented 2 years ago

Would it be relevant to be able to include the language setting?

I don't think it is. Since the language is only used to display the results, it does not impact how a test is run.

The first step is the design and specification of the URL schema.

This PR proposes a simple design for the URL schema that can be easily extended. Is this good enough to document this feature?

matsduf commented 2 years ago

Would it be relevant to be able to include the language setting?

I don't think it is. Since the language is only used to display the results, it does not impact how a test is run.

That is true, but the result is then displayed, and that should be in some language. If you create a link from the .fr registry to test your domain it would be reasonable to show the result by default in French.

We have actually already got a request for making it possible to include language setting in the URL to Zonemaster.

At least we should make sure we do not close the doors for other options such as language and settings for undelegated test. And that different options might be combined.

The first step is the design and specification of the URL schema.

This PR proposes a simple design for the URL schema that can be easily extended. Is this good enough to document this feature?

We already have /result/<hash-id> being used. That should be included in the documentation. Do we have anything else?

matsduf commented 2 years ago

You have not given any good reason to why the path should be domain_check/<domain> instead of check/<domain> more than it is simpler because it is there. It is of course better to have a short path and we have to think about the future and the future use.

The fact that we today have both result and test as paths shows why it is important to document and discussed before something is implemented.

I suggest that both test and domain_check are deprecated.

In the code it looks like history is also a path used by the GUI. Could you explain? Should that be part of the API? How does it work today?

I have proposed updates to the document in #335. I took the commit from this PR and added a change that would create an API that I think will work better for the future.

I assume that the implementation will be done in such a way that it will be possible to extend it with modifiers, e.g. ?lang=en, and also multipel modifiers, e.g. ?lang=en&ns=ns1.example.com. We have already had a request from a main user on being able to include language setting in the call.

ghost commented 2 years ago

We have already had a request from a main user on being able to include language setting in the call.

Could you point to this request? I can't find it.

In the code it looks like history is also a path used by the GUI. Could you explain?

Nope. It's been here since the beginning. Maybe there was an idea to make it possible to easily access a test history at that time already. Maybe something else. Who knows...

ghost commented 2 years ago

I've rebased and applied the suggestions from #335.

matsduf commented 2 years ago

We have already had a request from a main user on being able to include language setting in the call.

Could you point to this request? I can't find it.

I will try to find it. It was probably in a mail directly to me.

In the code it looks like history is also a path used by the GUI. Could you explain?

Nope. It's been here since the beginning. Maybe there was an idea to make it possible to easily access a test history at that time already. Maybe something else. Who knows...

OK. Thanks. It then seems like https://github.com/zonemaster/zonemaster-gui/blob/master/src/app/app.module.ts#L54 could be removed without any side effects. And I think it should.

matsduf commented 2 years ago

I've rebased and applied the suggestions from #335.

Do you think I should merge now or should we wait for more reviewers?

zonemaster / zonemaster-gui