Custom Bookmarks - Githubissues

JoelProminic commented 1 year ago

We would like to allow the user to create their own bookmarks, similar to the [access links]() from the Genesis App interface.

These links can be saved locally, but we want to support saving them to a Prominic database as well if we can link them to a Prominic server or customer.

TODO: Fill in details from @JustinProminic's email. I think I wrote this up in a separate project before, so I link that if I find it.

UPDATE: Justin requested that we support automatically adding bookmarks for database/applications imported from Moonshine (i.e. FormBuilder or Domino Visual Editor). This will be added as a separate step, but I should think about it as I make the design.

JoelProminic commented 1 year ago

This has been stuck for a while because I have concerns about using this interface for production Domino servers. I created a separate issue to address these concerns: #32.

I'll try to focus first on the local bookmarks for this. This shouldn't be too hard, but I see that the code is designed to convert JSON objects to the link configuration, so this will need some refactoring. I will probably want to create some POJOs to represent the links so that I can use the same code to convert local documents and remote JSON. The code is not trivial (see the test cases), so I definitely don't want to duplicate this.

JoelProminic commented 11 months ago

I have started working on the local agents and database updates in the features/issue_30_custom_bookmarks branch.

Rather than creating POJOs, I'm planning to generate JSON similar to the Genesis API and then process the links with the same code from GenesisRead. To support this, I refactored GenesisRead to move the relevant code to separate classes, LinkProcessor and JSONUtils (I should consider moving this to Moonshine-Domino-CRUD).

JoelProminic commented 11 months ago

@piotrzarzycki21, I have completed the initial version of the Bookmark API. I merged the features/issue_30_custom_bookmarks branch into main, since I was satisfied with how the implementation turned out (I was worreid it might be messier).

Here is a summary of the new actions. The generated documentation is here if I missed something.

All of these agents were added to the SuperHumanPortal.nsf database for now. We have discussed whether this needs to be in a separate database that can be more safely updated. You will need to deploy a new copy of the database (I'll send a compiled database by email).

If desired, you can generate the CRUD interface using Moonshine - I updated the Domino Visual Editor project with the new form. Some customizations will be needed, as discussed below.

Read

Agent: CustomBookmarkRead Example: https://domino.%server%/SuperHumanPortal.nsf/CustomBookmarkRead?OpenAgent

I designed the agent to return documents in the same format as the links sections from #13 and #19. The main additions are:

Property	Type	Description
DominoUniversalID	String	The generated unique ID, similar to what is used by the other CRUD agents
group	String	I added this to help us organize the links. The groups can be used to generate sections or new tree entries, depending on how we design it.

Example output:

{
  "documents": [
    {
      "server": "",
      "DominoUniversalID": "-snip-",
      "database": "",
      "view": "",
      "name": "TestURL",
      "type": "url",
      "url": "https://prominic.net",
      "group": "group1"
    },
    {
      "server": "-server-",
      "DominoUniversalID": "-snip-",
      "database": "names.nsf",
      "view": "",
      "name": "NamesDatabase",
      "nomadURL": "https://nomadweb.-server-/nomad/#/notes://-server-/names.nsf",
      "type": "database",
      "url": "notes://-server-/names.nsf",
      "group": "group1"
    },
    ...
  ],
  "errorMessage": "",
  "state": "authenticated",
  "common_name": "Anonymous",
  "username": "Anonymous",
  "status": "authenticated"
}

Create

Agent: CustomBookmarkCreate Examples:

Label	Parameter	Type	Multivalue	Required	Notes
Group (organization)	group	Text	false	false	Initially this can be a plain text field. Groups will be "added" automatically if a new name is used. If we want to pursue this further, we should provide a control which shows previously used groups, and possibly suggested (standard) groups.
Name	name	Text	false	false	the name of the link
Type	type	Text	false	false	The options are `database` (Label: "Database") and `browser` (Label: "URL" - we may change this later). `database` should be the default. The `type` should determine determines the other UI fields (not enforced by the agent)
type=browser
URL	url	Text	false	false
type=database
Server	server	Text	false	false	This should be left blank in most cases, which indicates the server that holds the SuperHumanPortal.nsf instance. When sharing links between servers, we could use this for links that are on every server. For SHI-generated servers, using other server names wil not work without more configuration. The UI should show a message for this like "Leave blank for current server"
Database	database	Text	false	false	The database name and path. TODO: Should we clarify the path part? I think this would be intuitive for users familiar with Domino/Notes, and I didn't have an immediate idea I liked for how to phrase this.
View (Optional)	view	Text	false	false

Update

Agent: CustomBookmarkUpdate Example: https://domino.%server%/SuperHumanPortal.nsf/CustomBookmarkUpdate?OpenAgent&DominoUniversalID=%id%&database=names.nsf

This uses the same properties as the CustomBookmarkCreate agent above.

Note that url will be updated for type=database links. Since this should not appear in the form, it can be ignored.

Also note that empty server values will be populated for type=database links returned by CustomBookmarkRead. I need to think more about how we can preserve the original value here - maybe I need to add another property for this.

Delete

Agent: CustomBookmarkUpdate Example: https://domino.%server%/SuperHumanPortal.nsf/CustomBookmarkDelete?OpenAgent&DominoUniversalID=%id%

This should be standard delete logic.

JoelProminic commented 11 months ago

Here is an example UI implementing the groups with divs/panels:

And here is an example with sections in the tree:

I don't especially love either design, so we'll probably want to do some refactoring/redesign as we do some practical tests. We can discuss this more tomorrow. (Tagging @JustinProminic so that he has a chance to review this in advance).

JoelProminic commented 11 months ago

We discussed this in the meeting today. @JustinProminic prefers the second design, with the groups displaying in the sidebar. @piotrzarzycki21, let us know if this will make the implementation significantly harder - in that case we can start with a simpler design and iterate.

piotrzarzycki21 commented 11 months ago

Here is an example UI implementing the groups with divs/panels:

And here is an example with sections in the tree:

I don't especially love either design, so we'll probably want to do some refactoring/redesign as we do some practical tests. We can discuss this more tomorrow. (Tagging @JustinProminic so that he has a chance to review this in advance).

@JoelProminic I'm progressing with implementation for second option and I don't understand based on design following:

What exactly mean "Delete" and "Edit" near buttons ? - Does this edit each particular like "Example Database Link" ?
Should I display one item from group in a way as it is right now in section "Installed Apps" ?

piotrzarzycki21 commented 11 months ago

Additional question: What is happening when I do "Add a Bookmark" ? I always thought that adding a bookmark is happening from list of application - I'm going to list of app and adding some of them to Bookmark.

JoelProminic commented 11 months ago

The Add a Bookmark button opens a forms with the properties from the Create agent. I included labels and other notes for the form in this table.

The resulting objects from this form are represented in the same way as links (#19). So, they can be a single button (type=url) or an expandable section (type=database). The Edit and Delete buttons are associated with each button/section. These should use icons instead of labels, and we might want to make them a bit smaller than the bookmark/link control.

These custom bookmarks will not necessarily be associated with Genesis Applications. Indeed, we should ideally have all the necessary links defined for the Genesis applications.

It seems there is some confusion about what is happening here, so let me give some context:

The user adds a new database using Moonshine and FormBuilder/DominoVisualEditor (bookmarks will potentially be automatically generated)
The user wants to save a link to the database. They click "Add a Bookmark", and fill in the properties associated with the Create action above (in this case, they select type=database).
The new bookmark appears in the selected group
The user wants to move this bookmark to a new group. They find the bookmark and click the corresponding Edit button, which shows a form similar to "Add a Bookmark"
The user can delete the bookmark if desired by finding the corresponding control and clicking the associated "Delete" button.

piotrzarzycki21 commented 11 months ago

URL in edit/add form should be capitalized
We shouldn't required view field and Group - default blank - something which we should discuss tomorrow's meeting - Initial group entry
Drop down list should be changed to radio buttons in edit/add form
Edit/Delete buttons after expanding item should stay near button

piotrzarzycki21 commented 11 months ago

@JoelProminic here is couple of screenshots with current state of implementatio:

https://github.com/Moonshine-IDE/Super.Human.Portal/assets/24554795/5f71f2c5-01c3-442d-b19d-1059cc9c314c

piotrzarzycki21 commented 11 months ago

@JoelProminic can we agree that if there is no Group/Bookmarks at all - user just installed Super Human Portal you will return me empty documents - Would that be correct assumption ?

piotrzarzycki21 commented 11 months ago

I have improved adding groups - new/empty etc. Currently if user won't fill "group" bookmarki will be added to group "Default"

piotrzarzycki21 commented 11 months ago

@JoelProminic All changes are available in newest build.

JoelProminic commented 11 months ago

I tested the last build

Test	Result
Create new bookmark in existing group	Works
Create new bookmark in new group	Works, group created automatically
Edit a bookmark to move it to an existing group	Works, new group displayed
Edit a bookmark to move it to a new group	BUG: old group displayed without moved entry, new group not added to sidebar
Delete the last bookmark in the group	Works, the empty group remains open
Delete all bookmarks	Works. After a refresh, a Default group is displayed
UPDATE: Edit a type=database bookmark and change the server and dbname	BUG: The buttons use the old server and database. After a refresh, I see the database was updated, but not the server. When I click the Open in Client button, it opens the original server and database. It looks like you are saving the `url` item here, which overrides the logic from my agent.

Some miscellaneous problems/notes:

The Open in Nomad button is missing for type=database links. You can see this in Installed App > Super.Human.Portal > Configuration Database. Is there some configuration missing from the agent?
I don't see a "Default" group if other groups exist (and there are no entries in the Default group). However, it does show up if there are no other groups, so I think this behavior makes sense. Some users could find it annoying if there was always an extra group they didn't want to use
New Groups are not sorted automatically. I refreshed the page, and the groups still weren't sorted

Overall this is looking pretty good, though. I'm going to do some further experiments with an practical task that @JustinProminic suggested, and I'll update this issue if I find more problems.

JoelProminic commented 11 months ago

Looking deeper into the case with editing the type=database fields, it looks like server is overridden in the CustomBookmarksRead agent to match the local server. I'm going to need to update this logic for custom bookmarks, but this shouldn't block the Royale work.

Since the url and nomadURL are generated by the agent, it makes sense that it wouldn't be updated automatically. We'll need to either refresh the agent after an edit, or add the updated JSON to the Update agent response. The latter is probably better, but I'll need to make a code update for this.

@piotrzarzycki21, the only change you should need to make for these type=database issues is to not save url when updating the logic. I may add some validation on the agent side, but the UI needs to be updated either way.

piotrzarzycki21 commented 11 months ago

@piotrzarzycki21, the only change you should need to make for these type=database issues is to not save url when updating the logic. I may add some validation on the agent side, but the UI needs to be updated either way.

I have made that change. When user select "database" url is being clear over submit, when user select url whatever was in "database" part will be also cleared.

piotrzarzycki21 commented 11 months ago

Edit a bookmark to move it to a new group BUG: old group displayed without moved entry, new group not added to sidebar

@JoelProminic I have fixed this issue as well - newest build should have all my changes.

piotrzarzycki21 commented 11 months ago

The Open in Nomad button is missing for type=database links. You can see this in Installed App > Super.Human.Portal > Configuration Database. Is there some configuration missing from the agent?

It was a bug on my sight. Fixed as well :)

piotrzarzycki21 commented 11 months ago

@JoelProminic I think you can deploy and retest everything again. I will move on to improve more SHI.

JoelProminic commented 11 months ago

The updates look good:

The Open in Nomad button exists and works as expected. Note that this will also need to be updated on an Update action, when I change the agent later today.
The UI update properly when I edit the bookmark to a different, existing group
I confirmed that the url is not saved for type=database bookmarks.

I started working on the agent updates, and hopefully they won't take long.

piotrzarzycki21 commented 11 months ago

Since the url and nomadURL are generated by the agent, it makes sense that it wouldn't be updated automatically. We'll need to either refresh the agent after an edit, or add the updated JSON to the Update agent response. The latter is probably better, but I'll need to make a code update for this.

Do you mean that to what I'm sending during update I should add nomadURL property value ? If that's what you mean it is happening already. This is what I'm sending during Add/Update -> https://github.com/Moonshine-IDE/Super.Human.Portal/blob/10e9f0594e814dc4f8ecc27da9acbcea6647f9d7/Super.Human.Portal_Royale/src/net/apacheRoyaleTemplatedApp/model/vo/BookmarkVO.as#L33

JoelProminic commented 11 months ago

I updated the CustomBookmarkUpdate agent to return document similar to what the Create agent does. @piotrzarzycki21, you should use this to update the object after the Update agent completes, at least for the updated url and nomadURL values (computed by the agent).

JoelProminic commented 11 months ago

I updated the code to support custom server values for custom bookmarks. This change is not enabled for GenesisRead.

Test	Result
GenesisRead	Only the agent server is used.
CustomBookmarkRead	Supports custom servers
CustomBookmarkCreate	Supports custom servers
CustomBookmarkUpdate	Supports custom servers

One issue that came up when reviewing the code was for nomadURL. This actually uses a server name twice:

https://nomadweb.%server%/nomad/#/notes://%server%/%database%/%view%

However, the first %server% determines where Nomad is opened, so I think it makes sense to always use the agent server.

I just pushed my updates for this. @piotrzarzycki21, you can deploy the agent updates using Run on Vagrant - there were no form/view updates required.

piotrzarzycki21 commented 11 months ago

I updated the CustomBookmarkUpdate agent to return document similar to what the Create agent does. @piotrzarzycki21, you should use this to update the object after the Update agent completes, at least for the updated url and nomadURL values (computed by the agent).

@JoelProminic In returned object I don't see dominoUniversalID which helps me determined what object should I update on my sight. Can you return it ?

JoelProminic commented 11 months ago

@piotrzarzycki21, I added DominoUniversalID to the response document. However, note that the returned DominoUniversalID property will always match the DominoUniversalID parameter in this case.

Let me know if you need a new copy of the database.

piotrzarzycki21 commented 11 months ago

@piotrzarzycki21, I added DominoUniversalID to the response document. However, note that the returned DominoUniversalID property will always match the DominoUniversalID parameter in this case.

Let me know if you need a new copy of the database.

I have made update on my sight. When you updated in edited parameter anything apart of Group I will updated on my sight one updated item, but if you change group I'm refreshing whole collection to rebuild group tree.

JoelProminic commented 10 months ago

We found that the text wasn't wrapping properly in Firefox. @piotrzarzycki21 made a quick update for this.

UPDATE: I retested this, but the result looks the same. I confirmed this in a private browsing window:

If this is going to take longer to fix, we can go ahead with the release. I still need to finish the updates related to #36, but that is not tied directly to the code in this project, so I can finish this after the tag and release are created.

JoelProminic commented 10 months ago

I moved the remaining formatting issue to #36 so that I could close the 0.1.7 milestone.

Moonshine-IDE / Super.Human.Portal

Custom Bookmarks #30

Read

Create

Update

Delete