feat: change `gnoweb` to add URL query string to render path #2876

jeronimoalbi · 2024-10-01T11:20:36Z

Description

This PR introduces a change for gnoweb to also pass the URL query string as part of the render path.

Passing the URL query string is required to support arguments when rendering realms, which is the case of the AVL pager implementation in #2584 that uses "page" and "size".

The PR changes the behavior of realm rendering calls by adding URL query arguments as suffix of the path argument, so URLs like https://gno.land/r/demo/foo:bar?baz=42 would call Render(path) with "bar?baz=42" as value of path.

It also changes how special gnoweb arguments like help of func are specified which now must be done by using the $ character, for example:

https://gno.land/r/demo/foo$help&func=Bar&name=Baz
https://gno.land/r/demo/foo:example$tz=Europe/Paris
https://gno.land/r/demo/foo:example?value=42$tz=Europe/Paris

Note that __func is now func, without the underscore prefix.

Possible Issues

The change could potentially affect realm or package implementations that rely on the render path in cases where the ? is not expected, for example when parsing or splitting it. Because of that there might be changes required to handle the case where a caller sends query string arguments. Realms and packages should be able to handle render paths which could look like render/path?arg=1&arg=N.

Links that still use ?, help and __func won't work as expected, they must be changed to use $ instead of ?, and func instead of __func.

Packages gno.land/p/moul/txlink and gno.land/p/moul/helplink has to be refactored to properly generate links.

codecov · 2024-10-01T11:24:31Z

Codecov Report

Attention: Patch coverage is 54.54545% with 25 lines in your changes missing coverage. Please review.

Project coverage is 63.06%. Comparing base (367408a) to head (3612d58).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
gno.land/pkg/gnoweb/gnoweb.go	54.54%	18 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2876      +/-   ##
==========================================
- Coverage   63.33%   63.06%   -0.28%     
==========================================
  Files         548      548              
  Lines       78601    81101    +2500     
==========================================
+ Hits        49780    51143    +1363     
- Misses      25466    26525    +1059     
- Partials     3355     3433      +78

Flag	Coverage Δ
contribs/gnodev	`61.11% <ø> (ø)`
contribs/gnofaucet	`14.82% <ø> (ø)`
gno.land	`67.18% <54.54%> (-0.19%)`	⬇️
gnovm	`67.88% <ø> (ø)`
misc/genstd	`79.72% <ø> (ø)`
tm2	`62.41% <ø> (+0.10%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gno.land/pkg/gnoweb/gnoweb.go

moul · 2024-10-01T19:07:34Z

Hey, what is the expected direct or indirect impact of this change?

Edit: found the answer here: #2584 (review)

jeronimoalbi · 2024-10-02T07:07:07Z

Hey, what is the expected direct or indirect impact of this change?

Edit: found the answer here: #2584 (review)

I will write a proper PR description soon and extend the unit tests for this case.
Also would like to check how it affects the mux package because I think it might break it.

moul · 2024-10-02T17:09:57Z

Related with #2602
Related with #2887

gno.land/pkg/gnoweb/gnoweb.go

@jeronimoalbi

This PR aimed to promote the use of a `p/` library for managing special help links from contracts. It also provided an opportunity for me to realize that our discussion about changing the `$` symbol would require some parsing and detection from the `gnoweb` perspective. If we want a simple library like this one, the goal should be to ideally craft a link to the current package without specifying the realm path. Relative URLs worked well with `?`, but they won't function with `$`. As an alternative, we can have this package look for `std.PrevRealm().PkgAddr` if it is not specified. cc @jeronimoalbi @thehowl @leohhhn Related with #2602 Related with #2876 --------- Signed-off-by: moul <[email protected]> Co-authored-by: Leon Hudak <[email protected]>

Public query arguments are the standard ones that come after the "?" symbol, while the private arguments come after the "$" symbol. Private gnoweb arguments are used for specific gnoweb features, some of which are `help` or `func` and are filtered from the render path when rendering realms.

leohhhn · 2024-11-05T08:54:11Z

I believe you missed the help button taking you to ?help instead of $help

In realm_render.html:

<a href="/r/{{ .Data.RealmName }}?help">[help]</a>

Co-authored-by: leohhhn <[email protected]>

leohhhn · 2024-11-05T09:07:41Z

And, for txlink, just switch out this line & fix the tests:

gno/examples/gno.land/p/moul/txlink/txlink.gno

Line 55 in 9096ef4

url := r.prefix() + "?help&__func=" + fn

I think that CTRL+F on examples/ should help you find other broken links, like ?help & __func

leohhhn

Good after fixing links in examples, we'll fix other broken links as we find them

misc/deployments/test4.gno.land/genesis.json

gno.land/pkg/gnoweb/gnoweb_test.go

moul

pre-approving, well done!

Co-authored-by: Manfred Touron <[email protected]>

leohhhn · 2024-11-06T12:41:37Z

Merging with a codecov drop because we will be scraping the whole gnoweb codebase soon; to be replaced with a full package refactor.

feat: change gnoweb to add URL query arguments to render path

9d833f3

github-actions bot assigned jeronimoalbi Oct 1, 2024

github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Oct 1, 2024

jeronimoalbi added 🌍 gnoweb Issues & PRs related to gnoweb and render and removed 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Oct 1, 2024

jeronimoalbi commented Oct 1, 2024

View reviewed changes

gno.land/pkg/gnoweb/gnoweb.go Outdated Show resolved Hide resolved

jeronimoalbi mentioned this pull request Oct 1, 2024

feat: add p/avl/pager #2584

Merged

jeronimoalbi changed the title ~~feat: change gnoweb to add URL query arguments to render path~~ feat: change gnoweb to add URL query string to render path Oct 2, 2024

moul mentioned this pull request Oct 2, 2024

[gnoweb/realms] Standardize hyperlink structure #2602

Open

moul mentioned this pull request Oct 2, 2024

feat: add p/moul/txlink + p/moul/helplink #2887

Merged

feat: unescape query args for the breadcrumb text

7269834

github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Oct 4, 2024

jeronimoalbi commented Oct 4, 2024

View reviewed changes

gno.land/pkg/gnoweb/gnoweb.go Outdated Show resolved Hide resolved

moul mentioned this pull request Oct 24, 2024

Dogfood r/boards on portal loop #3018

Open

leohhhn mentioned this pull request Oct 30, 2024

docs: add pagination pattern to Effective Gno #3044

Open

leohhhn self-requested a review October 30, 2024 18:13

jeronimoalbi and others added 5 commits October 31, 2024 17:34

feat: parse gnoweb args in in render handler and split parsing func

8f040ba

feat: change routes to support gnoweb query arguments

c78812c

chore: semantic changes

434acf2

Merge branch 'master' into devx/add-render-query-arguments

b2133a8

leohhhn marked this pull request as ready for review November 4, 2024 13:21

Merge branch 'master' into devx/add-render-query-arguments

6890a53

chore: fix invalid test help route

e33d776

fix: correct invalid help link

8d944ac

Co-authored-by: leohhhn <[email protected]>

leohhhn approved these changes Nov 5, 2024

View reviewed changes

jeronimoalbi added 3 commits November 5, 2024 10:23

refactor: update mould tx and help link packages

524364d

fix: correct help links in example realms

fb43558

fix: correct help links in genesis files

b03a814

github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Nov 5, 2024

Merge branch 'master' into devx/add-render-query-arguments

535b549

moul reviewed Nov 5, 2024

View reviewed changes

misc/deployments/test4.gno.land/genesis.json Show resolved Hide resolved

moul reviewed Nov 5, 2024

View reviewed changes

gno.land/pkg/gnoweb/gnoweb_test.go Show resolved Hide resolved

moul approved these changes Nov 5, 2024

View reviewed changes

jeronimoalbi and others added 2 commits November 6, 2024 10:20

test: add extra gnoweb routes test cases

d974783

Co-authored-by: Manfred Touron <[email protected]>

chore: remove invalid test case

3612d58

leohhhn merged commit 538ebff into gnolang:master Nov 6, 2024
131 of 132 checks passed

leohhhn deleted the devx/add-render-query-arguments branch November 6, 2024 12:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: change `gnoweb` to add URL query string to render path #2876

feat: change `gnoweb` to add URL query string to render path #2876

jeronimoalbi commented Oct 1, 2024 •

edited

Loading

codecov bot commented Oct 1, 2024 •

edited

Loading

moul commented Oct 1, 2024 •

edited

Loading

jeronimoalbi commented Oct 2, 2024

moul commented Oct 2, 2024

leohhhn commented Nov 5, 2024 •

edited

Loading

leohhhn commented Nov 5, 2024 •

edited

Loading

leohhhn left a comment

moul left a comment

leohhhn commented Nov 6, 2024

feat: change gnoweb to add URL query string to render path #2876

feat: change gnoweb to add URL query string to render path #2876

Conversation

jeronimoalbi commented Oct 1, 2024 • edited Loading

Description

Possible Issues

codecov bot commented Oct 1, 2024 • edited Loading

Codecov Report

moul commented Oct 1, 2024 • edited Loading

jeronimoalbi commented Oct 2, 2024

moul commented Oct 2, 2024

leohhhn commented Nov 5, 2024 • edited Loading

leohhhn commented Nov 5, 2024 • edited Loading

leohhhn left a comment

Choose a reason for hiding this comment

moul left a comment

Choose a reason for hiding this comment

leohhhn commented Nov 6, 2024

feat: change `gnoweb` to add URL query string to render path #2876

feat: change `gnoweb` to add URL query string to render path #2876

jeronimoalbi commented Oct 1, 2024 •

edited

Loading

codecov bot commented Oct 1, 2024 •

edited

Loading

moul commented Oct 1, 2024 •

edited

Loading

leohhhn commented Nov 5, 2024 •

edited

Loading

leohhhn commented Nov 5, 2024 •

edited

Loading