feat(caldav) Import caldav categories as Labels #1413

Merged
konrad merged 7 commits from ce72/api:1274_import_caldav_categories into main 2023-03-02 15:25:28 +00:00
Contributor

Resolves #1274

Resolves #1274
ce72 added 1 commit 2023-02-28 22:50:33 +00:00
ce72 changed title from 1274_import_caldav_categories to feat(caldav) Import caldav categories as Labels 2023-02-28 22:51:09 +00:00
ce72 changed title from feat(caldav) Import caldav categories as Labels to WIP: feat(caldav) Import caldav categories as Labels 2023-02-28 22:51:27 +00:00
ce72 force-pushed 1274_import_caldav_categories from 92415c1c38 to ce47ef1270 2023-03-01 06:40:52 +00:00 Compare
konrad reviewed 2023-03-01 10:10:20 +00:00
@ -197,0 +195,4 @@
return getLabelByExampleSimple(s, &Label{ID: labelID})
}
func getLabelByExampleSimple(s *xorm.Session, l *Label) (*Label, error) {
Owner

by example?

by example?
Author
Contributor

ok

ok
konrad marked this conversation as resolved
konrad requested changes 2023-03-01 10:13:46 +00:00
@ -276,0 +276,4 @@
for _, label := range vTask.Labels {
// Find or create Labels by title
err = label.ReadOne(s, vcls.user)
if err != nil {
Owner

Can you explicitley check for a LabelDoesNotExist error here?

Can you explicitley check for a LabelDoesNotExist error here?
Author
Contributor

ok

ok
konrad marked this conversation as resolved
@ -276,0 +282,4 @@
_ = s.Rollback()
return nil, err
}
err = label.ReadOne(s, vcls.user)
Owner

I think we don't need to read explicitely after creating the label.

I think we don't need to read explicitely after creating the label.
Author
Contributor

ok

ok
konrad marked this conversation as resolved
@ -318,6 +341,29 @@ func (vcls *VikunjaCaldavListStorage) UpdateResource(rpath, content string) (*da
return nil, errs.ForbiddenError
}
for _, label := range vTask.Labels {
Owner

This looks like it's duplicated, can you extract it in a new function?

This looks like it's duplicated, can you extract it in a new function?
Author
Contributor

ok

ok
konrad marked this conversation as resolved
ce72 force-pushed 1274_import_caldav_categories from 5824bc02ce to 2b5cc71809 2023-03-01 13:35:35 +00:00 Compare
ce72 force-pushed 1274_import_caldav_categories from 2b5cc71809 to 1cb1655c0d 2023-03-01 14:02:58 +00:00 Compare
ce72 added 1 commit 2023-03-01 16:43:55 +00:00
continuous-integration/drone/pr Build is passing Details
d2e97c36c8
feat(caldav): Import Labels from caldav categories
ce72 changed title from WIP: feat(caldav) Import caldav categories as Labels to feat(caldav) Import caldav categories as Labels 2023-03-01 16:55:10 +00:00
ce72 added 1 commit 2023-03-01 16:55:27 +00:00
continuous-integration/drone/pr Build is passing Details
8e093ca077
Merge branch 'main' into 1274_import_caldav_categories
konrad requested changes 2023-03-01 21:31:00 +00:00
konrad left a comment
Owner

Massive +1 for adding tests! That's awesome!

Massive +1 for adding tests! That's awesome!
@ -148,0 +156,4 @@
rec, c := testRequestSetup(t, method, payload, queryParams, urlParams)
c.Request().Header.Set(echo.HeaderContentType, echo.MIMETextPlain)
result, _ := caldav.BasicAuth(user.Username, "1234", c)
Owner

The password might get changed but it's completely fine to have it hardcoded in here - just a mental not to think of this in future.

The password might get changed but it's completely fine to have it hardcoded in here - just a mental not to think of this in future.
ce72 marked this conversation as resolved
@ -177,3 +177,3 @@
// @Router /labels/{id} [get]
func (l *Label) ReadOne(s *xorm.Session, a web.Auth) (err error) {
label, err := getLabelByIDSimple(s, l.ID)
label, err := getLabelSimple(s, l)
Owner

Please use getLabelByIDSimple.

Please use `getLabelByIDSimple`.
Author
Contributor

ok

ok
konrad marked this conversation as resolved
@ -370,0 +385,4 @@
func persistLabels(s *xorm.Session, a web.Auth, task *models.Task, labels []*models.Label) (err error) {
// Find or create Labels by title
for _, label := range labels {
err = label.ReadOne(s, a)
Owner

Can you use getLabelSimple here and only pass in the label's title?

Can you use `getLabelSimple` here and only pass in the label's title?
Author
Contributor

ok

ok
konrad marked this conversation as resolved
ce72 added 1 commit 2023-03-01 21:43:18 +00:00
continuous-integration/drone/pr Build is passing Details
3f7409aa56
Merge branch 'main' into 1274_import_caldav_categories
ce72 force-pushed 1274_import_caldav_categories from 764ef33fa4 to 1b3aa1b5a4 2023-03-01 22:48:14 +00:00 Compare
konrad approved these changes 2023-03-02 15:25:05 +00:00
konrad left a comment
Owner

Looks good now, thanks!

Looks good now, thanks!
konrad merged commit a62b57ac62 into main 2023-03-02 15:25:28 +00:00
konrad deleted branch 1274_import_caldav_categories 2023-03-02 15:25:28 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/vikunja#1413
No description provided.