Skip to content

Enhancement: extend planner task add with more options. Closes #3246#3315

Closed
Jwaegebaert wants to merge 7 commits intopnp:mainfrom
Jwaegebaert:plannerTaskAddMoreOptions
Closed

Enhancement: extend planner task add with more options. Closes #3246#3315
Jwaegebaert wants to merge 7 commits intopnp:mainfrom
Jwaegebaert:plannerTaskAddMoreOptions

Conversation

@Jwaegebaert
Copy link
Copy Markdown
Contributor

@Jwaegebaert Jwaegebaert commented May 17, 2022

Closes #3246

@Adam-it
Copy link
Copy Markdown
Member

Adam-it commented May 18, 2022

thanks @Jwaegebaert for all of your work including this amazing contribution 👍.
You rock 🤩
Will review it shortly

@martinlingstuyl martinlingstuyl self-assigned this May 19, 2022
martinlingstuyl
martinlingstuyl requested changes May 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@martinlingstuyl martinlingstuyl left a comment

Choose a reason for hiding this comment

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

Hi @Jwaegebaert, Great work to start with. I've got a few changes for you. 👍

Comment thread docs/docs/cmd/planner/task/task-add.md Outdated
Comment thread docs/docs/cmd/planner/task/task-add.md Outdated
Comment thread docs/docs/cmd/planner/task/task-add.md Outdated
Comment thread docs/docs/cmd/planner/task/task-add.md
Comment thread src/m365/planner/commands/task/task-add.ts
Comment thread src/m365/planner/commands/task/task-add.ts
@martinlingstuyl martinlingstuyl marked this pull request as draft May 19, 2022 12:03
@martinlingstuyl martinlingstuyl removed their assignment May 19, 2022
@plamber plamber requested a review from martinlingstuyl May 22, 2022 07:09
@martinlingstuyl martinlingstuyl self-assigned this May 22, 2022
@Jwaegebaert Jwaegebaert marked this pull request as ready for review May 23, 2022 08:01
martinlingstuyl
martinlingstuyl requested changes May 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@martinlingstuyl martinlingstuyl left a comment

Choose a reason for hiding this comment

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

Hi @Jwaegebaert,
Thanks for the rework! I've got two small last comments. Nothing to add further. Well done. 👍

Comment thread src/m365/planner/commands/task/task-add.ts Outdated
@Jwaegebaert Jwaegebaert mentioned this pull request May 26, 2022
Closed
@waldekmastykarz waldekmastykarz self-assigned this Jun 7, 2022
waldekmastykarz
waldekmastykarz requested changes Jun 7, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@waldekmastykarz waldekmastykarz left a comment

Choose a reason for hiding this comment

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

Very nicely done and we're almost there. Let's clarify a few things and do a few small adjustments before we merge it.

Comment thread src/m365/planner/commands/task/task-add.ts Outdated
Comment thread src/m365/planner/commands/task/task-add.ts Outdated
Comment thread src/m365/planner/commands/task/task-add.ts Outdated
Comment thread src/m365/planner/commands/task/task-add.ts Outdated
Comment thread src/m365/planner/commands/task/task-add.ts Outdated
Comment thread src/m365/planner/commands/task/task-add.ts Outdated
@waldekmastykarz waldekmastykarz marked this pull request as draft June 7, 2022 07:54
@waldekmastykarz waldekmastykarz removed their assignment Jun 7, 2022
@Jwaegebaert Jwaegebaert marked this pull request as ready for review June 7, 2022 14:03
@waldekmastykarz
Copy link
Copy Markdown
Member

waldekmastykarz commented Jun 8, 2022 •
edited
Loading

I've added a comment to #3340 to clarify the purpose of this change. Let's hold on with processing this PR until we clarified it there as we might be introducing unintended behavior. Let's mark this PR as ready for review once we concluded our discussion on the related issue.

@waldekmastykarz waldekmastykarz marked this pull request as draft June 8, 2022 17:58
@Jwaegebaert Jwaegebaert marked this pull request as ready for review June 17, 2022 13:44
martinlingstuyl
martinlingstuyl requested changes Jun 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@martinlingstuyl martinlingstuyl left a comment

Choose a reason for hiding this comment

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

Hi @Jwaegebaert, I'm on the verge of merging this in, but I am having some issues. Could you look into it?

Comment thread docs/docs/cmd/planner/task/task-add.md Outdated
Comment thread docs/docs/cmd/planner/task/task-add.md

```sh
m365 planner task add --title "My Planner Task" --planId "8QZEH7b3wkSbGQobscsM5gADCBa" --bucketId "IK8tuFTwQEa5vTonM7ZMRZgAKdna" --assignedToUserNames "[email protected],[email protected]" --asssigneePriority " !!"
```
Copy link
Copy Markdown
Contributor

@martinlingstuyl martinlingstuyl Jun 19, 2022

Choose a reason for hiding this comment

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

I also have an error surrounding the assigneePriority option here. I cannot execute this sample (even with single quotes):

Error: The request is invalid:
Specified order hint is in an invalid format: The input ( !!) contains an invalid exclamation point at index 2.

Copy link
Copy Markdown
Contributor Author

@Jwaegebaert Jwaegebaert Jun 19, 2022

Choose a reason for hiding this comment

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

That's weird, I could update the docs to use only a single exclamation !.

Copy link
Copy Markdown
Contributor

@martinlingstuyl martinlingstuyl Jun 19, 2022

Choose a reason for hiding this comment

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

Is the explanation correct then? And dont you have the same issue?

Copy link
Copy Markdown
Contributor Author

@Jwaegebaert Jwaegebaert Jun 19, 2022

Choose a reason for hiding this comment

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

Single one seems to be working.

afbeelding

Definitely not an expert in order hints, so I can't explain why the double one doesn't work. 😅

@martinlingstuyl martinlingstuyl marked this pull request as draft June 19, 2022 19:34
@Jwaegebaert Jwaegebaert marked this pull request as ready for review June 19, 2022 19:46
@martinlingstuyl
Copy link
Copy Markdown
Contributor

martinlingstuyl commented Jun 21, 2022

Merged manually, thank you 🤙👏

@Jwaegebaert Jwaegebaert deleted the plannerTaskAddMoreOptions branch October 2, 2022 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martinlingstuyl martinlingstuyl martinlingstuyl requested changes

@waldekmastykarz waldekmastykarz waldekmastykarz requested changes

Assignees

@martinlingstuyl martinlingstuyl

Labels

pr-merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enhancement: extend planner task add with more options

4 participants

@Jwaegebaert @Adam-it @waldekmastykarz @martinlingstuyl