Perform a test through the URL directly #334

ghost · 2022-07-19T10:00:11Z

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 · 2022-07-19T10:26:36Z

I also want to come back to comment in the issue (#200 (comment)): "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 · 2022-07-19T11:49:17Z

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?

docs/API.md

ghost · 2022-07-25T09:05:18Z

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...

New route "/check" Redirect "/domain_check" to "/check"

Redirect to "/result/<test-id>"

ghost · 2022-07-25T09:18:02Z

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

matsduf · 2022-07-25T09:43:40Z

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 · 2022-07-25T09:44:41Z

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

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

A new default path is used /check (instead of previous /domain_check)

matsduf

This looks fine. Tested, and all works as it should. I have only tested with a default base path, not with a custom path.

#335 should be merged first.

hannaeko

Nice feature (also thank you for removing the underscore in the url), I still have a couple of comments

src/app/components/form/form.component.ts

hannaeko · 2022-07-28T09:32:14Z

src/app/components/form/form.component.ts

+        this.domainName = params['domain'];
+        this.form.controls.domain.setValue(this.domainName);


Is the domainName property used anywhere else? If not would it make sense to remove it?

hannaeko

looks good to me

matsduf

#335 has been merged and as far as I can see this could be merged.

ghost added the T-Feature Type: New feature in software or test case description label Jul 19, 2022

ghost added this to the v2022.2 milestone Jul 19, 2022

ghost requested review from mattias-p, matsduf and hannaeko July 19, 2022 10:00

This comment was marked as resolved.

Sign in to view

ghost requested review from marc-vanderwal and tgreenx July 21, 2022 09:37

matsduf reviewed Jul 21, 2022

View reviewed changes

docs/API.md Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

ghost mentioned this pull request Jul 25, 2022

GUI API document #335

Merged

Alexandre Pion added 5 commits July 25, 2022 11:12

Deprecate "/domain_check" route

b93e27b

New route "/check" Redirect "/domain_check" to "/check"

Use route redirection for "/test/<test-id>"

8a5b8df

Redirect to "/result/<test-id>"

Fetch domain name from URL route parameter

9a55f79

Populate form input with domain from URL

a4c5a5f

Automatically run the test if a domain is provided

f296c36

Alexandre Pion added 4 commits July 25, 2022 11:49

Update href to new /check path

cc67b60

Fix e2e tests

43363e9

A new default path is used /check (instead of previous /domain_check)

Provide e2e redirection tests

8d0bb8a

Remove unused "/history" route

9b52c19

matsduf previously approved these changes Jul 25, 2022

View reviewed changes

hannaeko reviewed Jul 27, 2022

View reviewed changes

src/app/components/form/form.component.ts Outdated Show resolved Hide resolved

src/app/components/form/form.component.ts Outdated Show resolved Hide resolved

Fix implementation

80c046e

ghost dismissed matsduf’s stale review via 80c046e July 27, 2022 15:41

hannaeko reviewed Jul 28, 2022

View reviewed changes

Remove unused property

a473fb8

hannaeko approved these changes Jul 28, 2022

View reviewed changes

matsduf approved these changes Jul 28, 2022

View reviewed changes

matsduf approved these changes Aug 1, 2022

View reviewed changes

ghost merged commit 544d439 into zonemaster:develop Aug 19, 2022

ghost mentioned this pull request Sep 26, 2022

Rewording and provide new URL path /run-test instead of /check (former /domain_check) #339

Merged

tgreenx added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Dec 7, 2022

mattias-p mentioned this pull request Dec 16, 2022

Redirect deprecated path #376

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perform a test through the URL directly #334

Perform a test through the URL directly #334

ghost commented Jul 19, 2022 •

edited by ghost

Loading

This comment was marked as resolved.

matsduf commented Jul 19, 2022

ghost commented Jul 19, 2022

This comment was marked as resolved.

This comment was marked as resolved.

ghost commented Jul 25, 2022

ghost commented Jul 25, 2022

matsduf commented Jul 25, 2022

matsduf commented Jul 25, 2022

matsduf left a comment

hannaeko left a comment

hannaeko Jul 28, 2022 •

edited

Loading

hannaeko left a comment

matsduf left a comment

		this.domainName = params['domain'];
		this.form.controls.domain.setValue(this.domainName);

Perform a test through the URL directly #334

Perform a test through the URL directly #334

Conversation

ghost commented Jul 19, 2022 • edited by ghost Loading

Purpose

Context

Changes

How to test this PR

This comment was marked as resolved.

matsduf commented Jul 19, 2022

ghost commented Jul 19, 2022

This comment was marked as resolved.

This comment was marked as resolved.

ghost commented Jul 25, 2022

ghost commented Jul 25, 2022

matsduf commented Jul 25, 2022

matsduf commented Jul 25, 2022

matsduf left a comment

Choose a reason for hiding this comment

hannaeko left a comment

Choose a reason for hiding this comment

hannaeko Jul 28, 2022 • edited Loading

Choose a reason for hiding this comment

hannaeko left a comment

Choose a reason for hiding this comment

matsduf left a comment

Choose a reason for hiding this comment

ghost commented Jul 19, 2022 •

edited by ghost

Loading

hannaeko Jul 28, 2022 •

edited

Loading