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.

Sign up for GitHub

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

reel: migrate old tokens and update link flow to match #284

Merged
merged 1 commit into from
Sep 16, 2024
Merged

reel: migrate old tokens and update link flow to match #284

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions desk/app/reel.hoon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
open-describes=(set token:reel)
stable-id=(map cord token:reel)
==
++ flag ;~((glue fas) ;~(pfix sig fed:ag) sym)
:: url with old style token
++ url-for-token
|= [vic=cord token=cord]
Expand All @@ -79,7 +80,20 @@
=/ old !<(versioned-state old-state)
?- -.old
%4
`this(state old)
=. state old
=^ new-md stable-id
%+ roll
~(tap by our-metadata)
|= [[=token:reel =metadata:reel] [md=_our-metadata id=_stable-id]]
?^ (slaw %uv token) [md id]
?^ (rush token flag)
:- md
?: (~(has by id) token) id
(~(put by id) token token)
=/ new (rap 3 (scot %p our.bowl) '/' token ~)
:- (~(put by md) new metadata)
(~(put by id) new new)
Comment on lines +87 to +95
Copy link
Member

Choose a reason for hiding this comment

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

This is fine to run on every load, because none of the transformations result in follow-up:

  • New-style tokens (@uv format) are untouched,
  • "New old-style" tokens (~ship/term format) only guarantee they have an entry in stable-id,
  • "Old old-style" tokens get upgraded into "new old-style" tokens and put an entry in stable-id.

Correct?

Copy link
Member

Choose a reason for hiding this comment

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

(I'd still feel more comfortable with migration moving into state version %5, fwiw, just because it doesn't even risk repeated transforms.)

Copy link
Member Author

Choose a reason for hiding this comment

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

yep!

`this(our-metadata new-md)
%3
`this(state [%4 vic.old civ.old our-metadata.old outstanding-pokes.old ~ ~])
%2
Expand Down Expand Up @@ -174,18 +188,20 @@
%reel-want-token-link
=+ !<(=token:reel vase)
:_ this
=/ full-token
?^ (rush token flag) token
(rap 3 (scot %p our.bowl) '/' token ~)
=/ result=(unit [cord cord])
?. (~(has by our-metadata) token) `[token '']
`[token (url-for-token vic token)]
?. (~(has by our-metadata) full-token) `[full-token '']
`[full-token (url-for-token vic full-token)]
~[[%pass [%token-link-want token ~] %agent [src dap]:bowl %poke %reel-give-token-link !>(result)]]
%reel-give-token-link
=+ !<(result=(unit [cord cord]) vase)
?~ result `this
:_ this
=/ [token=cord url=cord] u.result
?: =('' url)
~[[%give %fact ~[[%token-link (scot %p src.bowl) token ~]] %json !>(~)]]
~[[%give %fact ~[[%token-link (scot %p src.bowl) token ~]] %json !>(s+url)]]
=/ path (stab (cat 3 '/token-link/' token))
Copy link
Member

Choose a reason for hiding this comment

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

Some smell on this not being /token-link/(scot %t token), but iirc that's for backcompat reasons that are out of scope here, right?

Copy link
Member

Choose a reason for hiding this comment

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

(Also some of the cords in the surrounding code should probably be token:reels.)

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm this should be able to work I think

Copy link
Member Author

Choose a reason for hiding this comment

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

nvm we can't do this

~[[%give %fact ~[path] %json !>(?:(=('' url) ~ s+url))]]
==
::
++ on-agent
Expand Down
Loading