-
Notifications
You must be signed in to change notification settings - Fork 0
Add the workspace API for CRUD, conversation related stuff #208
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please help test and verify the cascade deletions.
var instruction *string | ||
if w.Instruction != nil { | ||
value := *w.Instruction | ||
instruction = &value | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this? I feel it equals to :
workspace.Workspace{
...
Instruction: w.instruction,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:| I fix the issue with the instruction nil, and introduce that -)) but u r right -)) nothing happens here, let me remove
CreatedAt int64 `json:"created_at"` | ||
UpdatedAt int64 `json:"updated_at"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time.Time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hhmm, I have used it before =)) men, should have time to refactor them soon, but update to time.Time in this ticket now
trimmedName := strings.TrimSpace(name) | ||
if trimmedName == "" { | ||
return nil, common.NewErrorWithMessage("workspace name is required", "3a5dcb2f-9f1c-4f4b-8893-4a62f72f7a00") | ||
}50 | ||
if len([]rune(trimmedName)) > 50 { | ||
return nil, common.NewErrorWithMessage("workspace name is too long", "94a6a12b-d4f0-4594-8125-95de7f9ce3d6") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move these good validation to entity so we can use reproduce the same logic
func (w *Workspace) Nomorlize() error {
trimmedName := strings.TrimSpace(w.name)
if trimmedName == "" {
return nil, common.NewErrorWithMessage("workspace name is required", "3a5dcb2f-9f1c-4f4b-8893-4a62f72f7a00")
}
if len([]rune(trimmedName)) > 50 {
return nil, common.NewErrorWithMessage("workspace name is too long", "94a6a12b-d4f0-4594-8125-95de7f9ce3d6")
}
return error or nil
}
trimmedName := strings.TrimSpace(name) | ||
if trimmedName == "" { | ||
return nil, common.NewErrorWithMessage("workspace name is required", "3a5dcb2f-9f1c-4f4b-8893-4a62f72f7a00") | ||
}50 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
50?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's removed in next commit
return workspaces[0], nil | ||
} | ||
|
||
func (s *WorkspaceService) UpdateWorkspaceName(ctx context.Context, workspace *Workspace, name string) (*Workspace, *common.Error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about change to UpdateWorkspace(ctx context.Context, workspace *Workspace)
e := w.normalize()
if e == nil{
// give up
}
UpdateWorkspace(ctx, w)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split the Name and Instruction that Instruction can be huge text.
Name is minimal changes, we just split them to make everything lightweight
Select( | ||
query.Conversation.PublicID, | ||
query.Conversation.Title, | ||
query.Conversation.UserID, | ||
query.Conversation.WorkspacePublicID, | ||
query.Conversation.Status, | ||
query.Conversation.Metadata, | ||
query.Conversation.IsPrivate, | ||
). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gorm faces the issue which update NULL or remove value, we have to load and update it implicit.
In this case, I want to update WorkspacePublicID as nil when user want to move conversation out of workspace scope
workspace.Name = updated.Name | ||
workspace.Instruction = updated.Instruction | ||
workspace.UpdatedAt = updated.UpdatedAt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just
workspace = updatedModel.EtoD()
But I feel like I'm committing a crime by overwriting function arguments...
or return updatedModel.EtoD() to indicate that is a new entity
func (r *WorkspaceGormRepository) Update(ctx context.Context, workspace *domain.Workspace) (*domain.Workspace error) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We return the update value, that code is ridiculous too.
workspace.UpdatedAt = updated.UpdatedAt
is enough
I fix that
func (r *WorkspaceGormRepository) FindByID(ctx context.Context, id uint) (*domain.Workspace, error) { | ||
tx := r.db.GetTx(ctx).WithContext(ctx) | ||
var model dbschema.Workspace | ||
if err := tx.Where("id = ?", id).First(&model).Error; err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"id = ?" Avoid using strings in statements.
if err := tx.Where("public_id = ?", publicID).First(&model).Error; err != nil { | ||
return nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"public_id = ?" Avoid using strings in statements.
if filter.UserID != nil { | ||
tx = tx.Where("user_id = ?", *filter.UserID) | ||
} | ||
if filter.PublicID != nil { | ||
tx = tx.Where("public_id = ?", *filter.PublicID) | ||
} | ||
if filter.PublicIDs != nil && len(*filter.PublicIDs) > 0 { | ||
tx = tx.Where("public_id IN ?", *filter.PublicIDs) | ||
} | ||
if filter.IDs != nil && len(*filter.IDs) > 0 { | ||
tx = tx.Where("id IN ?", *filter.IDs) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using strings in statements.
Add new entity:
Swagger: https://api-dev.jan.ai/api/swagger/index.html#/
New APIs:
1. Create a Workspace
POST https://api.jan.ai/v1/conv/workspaces
max length of workspace is 50
2. List Workspaces
GET https://api.jan.ai/v1/conv/workspaces
3. Update Workspace Name with patch Request
PATCH https://api.jan.ai/v1/conv/workspaces/{workspace_id}
4. Update Workspace Instruction
PATCH https://api.jan.ai/v1/conv/workspaces/{workspace_id}/instruction
5. Create a Conversation in a Workspace
POST https://api.jan.ai/v1/conversations
{
"title": "Enterprise Onboarding",
"workspace_id": "ws_abcd1234",
}
workspace_id
attaches the new conversation to the supplied workspace.6. List Conversations in a Workspace
GET https://api.jan.ai/v1/conv/conversations?workspace_id=
7. Move (or Remove) a Conversation Workspace Association
PATCH https://api.jan.ai/v1/conversations/{conversation_id}/workspace
{
"workspace_id": "ws_newworkspace42"
}
Provide a different workspace public ID to move the conversation. Use
"workspace_id": null
to detach it from all workspaces.8. Delete a Workspace via conv Route (also Cascade)
DELETE https://api.jan.ai/v1/conv/workspaces/{workspace_id}