Monorepo for Tangled tangled.org

appview: allows a default knot to be configured #991

open opened by willdot.net targeting master from willdot.net/tangled-fork: default-knot

This follows on from the work carried out in #836

I've added a select box in the Knots settings page which pulls in the users knots and also adds in knot1.tangled.sh. When the user selects one of these options, it will save to their profile in the database. NOTE: I haven't yet implemented adding that to the AT record because I'm not sure on how the lexicon setup works yet!

Then when users go to create a new repo / fork, if there is a value in their profile for the default knot, then that will pre select the knot to use for the new repo / fork.

This is a duplicate of https://tangled.org/tangled.org/core/pulls/858/ which had to be closed because there were merge conflicts which couldn't be resolved due to the origin fork being deleted ๐Ÿ™ˆ

Labels

None yet.

assignee

None yet.

Participants 2
AT URI
at://did:plc:dadhhalkfcq3gucaq25hjqon/sh.tangled.repo.pull/3mcrzfonnxs22
+156 -9
Diff #10
+8
appview/db/db.go
··· 596 foreign key (webhook_id) references webhooks(id) on delete cascade 597 ); 598 599 create table if not exists migrations ( 600 id integer primary key autoincrement, 601 name text unique
··· 596 foreign key (webhook_id) references webhooks(id) on delete cascade 597 ); 598 599 + create table if not exists knot_preferences ( 600 + id integer primary key autoincrement, 601 + user_did text not null unique, 602 + default_knot text, 603 + 604 + foreign key (user_did, default_knot) references registrations(did, domain) on delete cascade 605 + ); 606 + 607 create table if not exists migrations ( 608 id integer primary key autoincrement, 609 name text unique
+35
appview/db/preferences.go
··· 50 51 return nil 52 }
··· 50 51 return nil 52 } 53 + 54 + func GetKnotPreference(e Execer, did string) (*models.KnotPreference, error) { 55 + var knotPreference models.KnotPreference 56 + 57 + err := e.QueryRow( 58 + `select id, user_did, default_knot from knot_preferences where user_did = ?`, 59 + did, 60 + ).Scan(&knotPreference.ID, &knotPreference.Did, &knotPreference.DefaultKnot) 61 + if err == sql.ErrNoRows { 62 + return nil, nil 63 + } 64 + 65 + if err != nil { 66 + return nil, err 67 + } 68 + 69 + return &knotPreference, nil 70 + } 71 + 72 + func UpsertKnotPreference(e Execer, did, defaultKnot string) error { 73 + _, err := e.Exec( 74 + `insert or replace into knot_preferences ( 75 + user_did, 76 + default_knot 77 + ) 78 + values (?, ?)`, 79 + did, 80 + defaultKnot, 81 + ) 82 + if err != nil { 83 + return err 84 + } 85 + 86 + return nil 87 + }
+20 -2
appview/knots/knots.go
··· 68 return 69 } 70 71 k.Pages.Knots(w, pages.KnotsParams{ 72 - LoggedInUser: user, 73 - Registrations: registrations, 74 }) 75 } 76
··· 68 return 69 } 70 71 + availableKnots, err := k.Enforcer.GetKnotsForUser(user.Did()) 72 + if err != nil { 73 + k.Logger.Error("failed to fetch available knots for user", "err", err) 74 + w.WriteHeader(http.StatusInternalServerError) 75 + return 76 + } 77 + 78 + defaultKnot := "" 79 + knotPrefence, err := db.GetKnotPreference(k.Db, user.Did()) 80 + if err != nil { 81 + k.Logger.Warn("gettings users knot preferences", "error", err) 82 + } 83 + if knotPrefence != nil { 84 + defaultKnot = knotPrefence.DefaultKnot 85 + } 86 + 87 k.Pages.Knots(w, pages.KnotsParams{ 88 + LoggedInUser: user, 89 + Registrations: registrations, 90 + AvailableKnots: availableKnots, 91 + DefaultKnot: defaultKnot, 92 }) 93 } 94
+6
appview/models/preferences.go
··· 6 HideMine bool 7 HideOthers bool 8 }
··· 6 HideMine bool 7 HideOthers bool 8 } 9 + 10 + type KnotPreference struct { 11 + ID int 12 + Did string 13 + DefaultKnot string 14 + }
+7 -3
appview/pages/pages.go
··· 440 } 441 442 type KnotsParams struct { 443 - LoggedInUser *oauth.MultiAccountUser 444 - Registrations []models.Registration 445 - Tab string 446 } 447 448 func (p *Pages) Knots(w io.Writer, params KnotsParams) error { ··· 506 type NewRepoParams struct { 507 LoggedInUser *oauth.MultiAccountUser 508 Knots []string 509 } 510 511 func (p *Pages) NewRepo(w io.Writer, params NewRepoParams) error { ··· 516 LoggedInUser *oauth.MultiAccountUser 517 Knots []string 518 RepoInfo repoinfo.RepoInfo 519 } 520 521 func (p *Pages) ForkRepo(w io.Writer, params ForkRepoParams) error {
··· 440 } 441 442 type KnotsParams struct { 443 + LoggedInUser *oauth.MultiAccountUser 444 + Registrations []models.Registration 445 + Tab string 446 + AvailableKnots []string 447 + DefaultKnot string 448 } 449 450 func (p *Pages) Knots(w io.Writer, params KnotsParams) error { ··· 508 type NewRepoParams struct { 509 LoggedInUser *oauth.MultiAccountUser 510 Knots []string 511 + DefaultKnot string 512 } 513 514 func (p *Pages) NewRepo(w io.Writer, params NewRepoParams) error { ··· 519 LoggedInUser *oauth.MultiAccountUser 520 Knots []string 521 RepoInfo repoinfo.RepoInfo 522 + DefaultKnot string 523 } 524 525 func (p *Pages) ForkRepo(w io.Writer, params ForkRepoParams) error {
+28
appview/pages/templates/knots/index.html
··· 31 <div class="flex flex-col gap-6"> 32 {{ block "list" . }} {{ end }} 33 {{ block "register" . }} {{ end }} 34 </div> 35 </section> 36 {{ end }} ··· 62 </section> 63 {{ end }} 64 65 {{ define "register" }} 66 <section class="rounded w-full lg:w-fit flex flex-col gap-2"> 67 <h2 class="text-sm font-bold py-2 uppercase dark:text-gray-300">register a knot</h2>
··· 31 <div class="flex flex-col gap-6"> 32 {{ block "list" . }} {{ end }} 33 {{ block "register" . }} {{ end }} 34 + {{ block "default-knot" . }} {{ end }} 35 </div> 36 </section> 37 {{ end }} ··· 63 </section> 64 {{ end }} 65 66 + {{ define "default-knot" }} 67 + <section class="rounded w-full flex flex-col gap-2"> 68 + <h2 class="text-sm font-bold py-2 uppercase dark:text-gray-300">default knot</h2> 69 + <form hx-post="/profile/default-knot" class="col-span-1 md:col-span-1 md:justify-self-end group flex gap-2 items-stretch"> 70 + <select 71 + id="default-knot" 72 + name="default-knot" 73 + required 74 + class="p-1 max-w-64 border border-gray-200 bg-white dark:bg-gray-800 dark:text-white dark:border-gray-700"> 75 + {{/* For some reason, we can't use an empty string in a <select> in all scenarios unless it is preceded by a disabled select?? No idea, could just be a Firefox thing? */}} 76 + <option value="[[none]]" class="py-1" {{ if not $.DefaultKnot }}selected{{ end }}> 77 + Choose a default knot 78 + </option> 79 + {{ range $.AvailableKnots }} 80 + <option value="{{ . }}" class="py-1" {{ if eq . $.DefaultKnot }}selected{{ end }}> 81 + {{ . }} 82 + </option> 83 + {{ end }} 84 + </select> 85 + <button class="btn flex gap-2 items-center" type="submit"> 86 + {{ i "check" "size-4" }} 87 + {{ i "loader-circle" "w-4 h-4 animate-spin hidden group-[.htmx-request]:inline" }} 88 + </button> 89 + </form> 90 + </section> 91 + {{ end }} 92 + 93 {{ define "register" }} 94 <section class="rounded w-full lg:w-fit flex flex-col gap-2"> 95 <h2 class="text-sm font-bold py-2 uppercase dark:text-gray-300">register a knot</h2>
+3 -1
appview/pages/templates/repo/fork.html
··· 25 value="{{ . }}" 26 class="mr-2" 27 id="domain-{{ . }}" 28 - {{if eq (len $.Knots) 1}}checked{{end}} 29 /> 30 <label for="domain-{{ . }}" class="dark:text-white">{{ . }}</label> 31 </div>
··· 25 value="{{ . }}" 26 class="mr-2" 27 id="domain-{{ . }}" 28 + {{if eq (len $.Knots) 1}}checked 29 + {{else if eq $.DefaultKnot . }}checked 30 + {{end}} 31 /> 32 <label for="domain-{{ . }}" class="dark:text-white">{{ . }}</label> 33 </div>
+3 -1
appview/pages/templates/repo/new.html
··· 156 class="mr-2" 157 id="domain-{{ . }}" 158 required 159 - {{if eq (len $.Knots) 1}}checked{{end}} 160 /> 161 <label for="domain-{{ . }}" class="dark:text-white lowercase">{{ . }}</label> 162 </div>
··· 156 class="mr-2" 157 id="domain-{{ . }}" 158 required 159 + {{if eq (len $.Knots) 1}}checked 160 + {{else if eq $.DefaultKnot . }}checked 161 + {{end}} 162 /> 163 <label for="domain-{{ . }}" class="dark:text-white lowercase">{{ . }}</label> 164 </div>
+10
appview/repo/repo.go
··· 1004 return 1005 } 1006 1007 rp.pages.ForkRepo(w, pages.ForkRepoParams{ 1008 LoggedInUser: user, 1009 Knots: knots, 1010 RepoInfo: rp.repoResolver.GetRepoInfo(r, user), 1011 }) 1012 1013 case http.MethodPost:
··· 1004 return 1005 } 1006 1007 + defaultKnot := "" 1008 + knotPrefence, err := db.GetKnotPreference(rp.db, user.Did()) 1009 + if err != nil { 1010 + rp.logger.Warn("gettings users knot preferences", "error", err) 1011 + } 1012 + if knotPrefence != nil { 1013 + defaultKnot = knotPrefence.DefaultKnot 1014 + } 1015 + 1016 rp.pages.ForkRepo(w, pages.ForkRepoParams{ 1017 LoggedInUser: user, 1018 Knots: knots, 1019 RepoInfo: rp.repoResolver.GetRepoInfo(r, user), 1020 + DefaultKnot: defaultKnot, 1021 }) 1022 1023 case http.MethodPost:
+23
appview/state/profile.go
··· 649 s.updateProfile(profile, w, r) 650 } 651 652 func (s *State) updateProfile(profile *models.Profile, w http.ResponseWriter, r *http.Request) { 653 user := s.oauth.GetMultiAccountUser(r) 654 tx, err := s.db.BeginTx(r.Context(), nil)
··· 649 s.updateProfile(profile, w, r) 650 } 651 652 + func (s *State) UpdateDefaultKnotPreference(w http.ResponseWriter, r *http.Request) { 653 + err := r.ParseForm() 654 + if err != nil { 655 + log.Println("invalid preference update form", err) 656 + return 657 + } 658 + user := s.oauth.GetMultiAccountUser(r) 659 + 660 + defaultKnot := r.Form.Get("default-knot") 661 + 662 + if defaultKnot == "[[none]]" { // see pages/templates/knots/index.html for more info on why we use this value 663 + defaultKnot = "" 664 + } 665 + 666 + err = db.UpsertKnotPreference(s.db, user.Did(), defaultKnot) 667 + if err != nil { 668 + log.Println("failed to update default knot preference", err) 669 + return 670 + } 671 + 672 + s.pages.HxRefresh(w) 673 + } 674 + 675 func (s *State) updateProfile(profile *models.Profile, w http.ResponseWriter, r *http.Request) { 676 user := s.oauth.GetMultiAccountUser(r) 677 tx, err := s.db.BeginTx(r.Context(), nil)
+1
appview/state/router.go
··· 168 r.Post("/avatar", s.UploadProfileAvatar) 169 r.Delete("/avatar", s.RemoveProfileAvatar) 170 r.Post("/punchcard", s.UpdateProfilePunchcardSetting) 171 }) 172 173 r.Mount("/settings", s.SettingsRouter())
··· 168 r.Post("/avatar", s.UploadProfileAvatar) 169 r.Delete("/avatar", s.RemoveProfileAvatar) 170 r.Post("/punchcard", s.UpdateProfilePunchcardSetting) 171 + r.Post("/default-knot", s.UpdateDefaultKnotPreference) 172 }) 173 174 r.Mount("/settings", s.SettingsRouter())
+10
appview/state/state.go
··· 430 return 431 } 432 433 s.pages.NewRepo(w, pages.NewRepoParams{ 434 LoggedInUser: user, 435 Knots: knots, 436 }) 437 438 case http.MethodPost:
··· 430 return 431 } 432 433 + defaultKnot := "" 434 + knotPrefence, err := db.GetKnotPreference(s.db, user.Did()) 435 + if err != nil { 436 + s.logger.Warn("gettings users knot preferences", "error", err) 437 + } 438 + if knotPrefence != nil { 439 + defaultKnot = knotPrefence.DefaultKnot 440 + } 441 + 442 s.pages.NewRepo(w, pages.NewRepoParams{ 443 LoggedInUser: user, 444 Knots: knots, 445 + DefaultKnot: defaultKnot, 446 }) 447 448 case http.MethodPost:
+2 -2
flake.lock
··· 99 "lastModified": 1731402384, 100 "narHash": "sha256-OwUmrPfEehLDz0fl2ChYLK8FQM2p0G1+EMrGsYEq+6g=", 101 "type": "tarball", 102 - "url": "https://github.com/IBM/plex/releases/download/@ibm/plex-mono@1.1.0/ibm-plex-mono.zip" 103 }, 104 "original": { 105 "type": "tarball", 106 - "url": "https://github.com/IBM/plex/releases/download/@ibm/plex-mono@1.1.0/ibm-plex-mono.zip" 107 } 108 }, 109 "indigo": {
··· 99 "lastModified": 1731402384, 100 "narHash": "sha256-OwUmrPfEehLDz0fl2ChYLK8FQM2p0G1+EMrGsYEq+6g=", 101 "type": "tarball", 102 + "url": "https://github.com/IBM/plex/releases/download/@ibm%2Fplex-mono@1.1.0/ibm-plex-mono.zip" 103 }, 104 "original": { 105 "type": "tarball", 106 + "url": "https://github.com/IBM/plex/releases/download/@ibm%2Fplex-mono@1.1.0/ibm-plex-mono.zip" 107 } 108 }, 109 "indigo": {

History

13 rounds 17 comments
sign up or login to add to the discussion
1 commit
expand
appview: allows a default knot to be configured
merge conflicts detected
expand
  • appview/db/db.go:596
expand 0 comments
1 commit
expand
appview: allows a default knot to be configured
expand 0 comments
1 commit
expand
appview: allows a default knot to be configured
expand 0 comments
1 commit
expand
appview: allows a default knot to be configured
expand 0 comments
1 commit
expand
appview: allow default knot to be saved as a preference and then used when creating a new repo
expand 0 comments
1 commit
expand
appview: allow default knot to be saved as a preference and then used when creating a new repo
expand 8 comments

sorry for the late review! on the whole the code looks good. my only nit is with the model: we are storing the default knot as a string, but knot deletions can make this go out of sync. i think an FK relationship with the knot registrations table could fix this. we just unset defaults when a knot is deleted.

That's a super valid point, didn't think of that!

I'll add a default value to empty string on the default_knot field, a foreign key constraint to the registrations table domain field since it's a unique field, and then ON DELETE SET DEFAULT so that if the knot gets deleted, the knot preference gets set to an empty string.

That's a super valid point, didn't think of that!

I'll add a default value to empty string on the default_knot field, a foreign key constraint to the registrations table domain field since it's a unique field, and then ON DELETE SET DEFAULT so that if the knot gets deleted, the knot preference gets set to an empty string.

sure yeah that would work, we could cascade as well. either is fine i think.

I thought about cascade but my understanding is that the knot_preference row for the user would be deleted if the knot is deleted.

If this is a preferences row for a user, there has potential for other things to be stored in that row which we may not want deleted on knot deletion.

makes sense!

Ok I've made these changes but I have had to do the cascade action on delete.

The reason for this is that the registrations table has a unique constraint on both the did and domain fields. The did is the user that is adding the registration, so that should be the user that is also creating the knot_preference row (which is also unique) and it can't be null.

Using the on delete set default therefor wouldn't work because the user_did field on the knot_preference can't be null.

So cascade is the only option.

Although now I'm starting to doubt the design of the table. Perhaps it should be just for default knot preferences and thus named as that?

1 commit
expand
appview: allow default knot to be saved as a preference and then used when creating a new repo
expand 0 comments
1 commit
expand
appview: allow default knot to be saved as a preference and then used when creating a new repo
expand 0 comments
1 commit
expand
appview: allows a default knot to be selected from knots available to the user
expand 5 comments

@oppi.li While attempting to implement something else related to profiles, I've noticed that UpsertProfile(...) herehttps://tangled.org/tangled.org/core/blob/master/appview/db/profile.go#L133 also gets called as part of a JS ingester here https://tangled.org/tangled.org/core/blob/master/appview/ingester.go#L352 which means the DefaultKnot field will be overwritten with an empty string when a profile event is ingested. Am I understanding that correctly?

So instead I should probably not use the UpsertProfile(...) but use a smaller scoped function that just sets the Default Knot field?

ah that is correct, the default knot will be overwritten by that call. i suppose that is one of the drawbacks of overloading models.Profile! instead of cloning UpsertProfile to add this as an exception, perhaps this is an indication that we need to avoid using the profile table/model for this. what do you think about creating a preferences table for stuff like this? we already have notification_preferences, we could do something similar for knots.

i see you have a PR open for punchcard work as well, which would fit quite well in a punchcard_preferences table.

Yh I'm 100% down for using that approach. While I was implementing the punchcard PR I had this nagging feeling in me that this and other things belonged in some sort of separate "preferences" table so splitting them out into things like knot_preferences or punchcard_preferences is the right answer for me.

I'll redo that for this PR and then do the same of the other PR and hopefully it will prevent the merge conflicts I was dreading ๐Ÿ™ˆ

yeah! knot_preferences and punchcard_preferences seems okay for now. thanks for keeping at this by the way, and sorry for the slow review cycle!

No worries! I don't expect immediate reviews ๐Ÿ˜

I've refactored to use a new knot_preferences table for this PR. I had some merge conflicts with master though, so rebased that although took me a couple attempts to get everything nice and neat, hence so many rounds ๐Ÿ™ˆ

2 commits
expand
appview: allows a default knot to be configured
appview: allows a default knot to be selected from knots available to the user
expand 0 comments
1 commit
expand
appview: allows a default knot to be configured
expand 2 comments

the code itself works pretty nicely now. so i've noticed a couple issues (apologies for not looking more closely earlier!):

  • the default-knot selector only iterates over knots that the user is an admin of, whereas they should be able to choose any knot that they are a member of as the default. you can use s.enforcer.GetKnotsForUser(...) to get a list of knots that a user has access to
  • the HTML change adds knot1.tangled.sh as an explicit case, this is not necessary if the above is implemented
  • the HTML template currently triggers a request when the select changes, could we make this more like how the spindle selector works? a form with a button to submit.

I was looking for ages trying to find somewhere that did something similar. I don't use spindles so never saw that ๐Ÿ™ˆ A much better approach.

Also good shout on the knots for a user. Reading the code now, I understand what the registrations are now.

Anyhow, I've updated the PR and squashed everything so it should be good to test now.

Please note, that I have yet to figure out how to get multiple Knots running locally, and since I don't get knot1.tangled.sh locally I had to hard code some things in to test it worked, which is less than ideal.

1 commit
expand
appview: allows a default knot to be configured
expand 0 comments
1 commit
expand
appview: allows a default knot to be configured
expand 2 comments

i get this error at runtime:

2026/01/20 04:58:12 profile err 5 values for 6 columns
2026/01/20 04:58:12 failed to update profile 5 values for 6 columns

this sql query is incorrect:

		`insert or replace into profile (
			did,
			description,
			include_bluesky,
			location,
			pronouns,
			default_knot
		)
		values (?, ?, ?, ?, ?)`,

there should be 6 ? placeholders!

Ah crap, someone needs to create a lint tool that detects that ๐Ÿคฆโ€โ™‚๏ธ

Fixed.