search

pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
920 stars 326 forks source link

Bug: 'spo file add' removing leading zeros #702

Closed ohthreesixtyfive closed 5 years ago

ohthreesixtyfive commented 5 years ago

Description:

When adding a file and attempting to set a field (in this case a single-line of text), a value passed through with leading zeros will have those zeros removed.

Steps to reproduce:

spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path 'C:\MS365.jpg' --Title "000123"

Expected:

New file added to destination with a Title attribute of '000123'

Actual:

New file added to destination with a Title attribute of '123'

Additional notes (Updated: 12/6/2018):

After further testing, this only appears to affect values that contain only numerical characters. If any characters other than a number are included in the value (regardless of positioning), the leading zeros are preserved.

Therefore, the following will create a new item with the 'Title' attribute of ' 000123' as expected.

spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path 'C:\MS365.jpg' --Title " 000123"

Likewise, the following will create a new item with the 'Title' attribute of '000123 ABCD' as expected.


spo file add --webUrl https://contoso.sharepoint.com/sites/project-x --folder 'Shared Documents' --path 'C:\MS365.jpg' --Title "000123 ABCD"
waldekmastykarz commented 5 years ago

Thanks for bringing it up! I'll have a look into it and hopefully we'll be able to fix it quickly. Sorry for the trouble.

waldekmastykarz commented 5 years ago

@ohthreesixtyfive following is a note-to-self based on my investigation for documentation purposes. Nothing you need to handle or respond to 🙂

I did some investigation and the reported behavior is caused by minimist deducting the type of args values unless they're explicitly set to strings. Unfortunately, for commands that allow unknown args, we can't know the names of all string args upfront and the types of args are specified in vorpal when instantiating the command rather than when executing it.

One reasonable way is to implement a workaround where the passed args are parsed again as the first step of the command execution and the list of args that should be handled as strings is dynamically constructed from the specified unknown args. That way we can create a new args object with the types of unknown args handled as expected.

VelinGeorgiev commented 5 years ago

Just as reference: we might expect similar behavior for commands 'spo listitem add' and update.

waldekmastykarz commented 5 years ago

Basically, any command that allows unknown properties could have this issue. I've been thinking about fixing it in the SpoCommand or maybe even Command class so that it's automatically fixed without us having to worry about it in specific commands. Let's see if it will work out.

ohthreesixtyfive commented 5 years ago

Thanks for bringing it up! I'll have a look into it and hopefully we'll be able to fix it quickly. Sorry for the trouble.

My pleasure! Major thanks to you and the other contributors for developing this as I've been able to use it on a major migration from an aging ECM to our SharePoint site.

Originally, I was using SharePointPnP PowerShell (Add-PnPFile), but was facing excessive memory usage issues when the amount of items being written was too large. Your very timely release of 'spo add item', which has not exhibited the same issue, has been greatly appreciated. :)

For the time being, I simply appended a space (' ') to the values that this was affecting - not too big of an issue (and something I can easily clean up later on).

I'm looking forward to building more solutions using this CLI.

Best regards,

Ramiro

waldekmastykarz commented 5 years ago

Thank you for the kind words and great to hear we could help you. Please, never hesitate to reach out if you find anything out of the ordinary or if we can help with anything 😊

waldekmastykarz commented 5 years ago

@ohthreesixtyfive #710 should be fixing the behavior you're seeing. If you can, could you please verify it before we merge it? Otherwise, the fix will come in the next beta and you'll be able to test it then. Once again, thanks for bringing it up.

ohthreesixtyfive commented 5 years ago

@ohthreesixtyfive #710 should be fixing the behavior you're seeing. If you can, could you please verify it before we merge it? Otherwise, the fix will come in the next beta and you'll be able to test it then. Once again, thanks for bringing it up.

I'd be happy to test it. I'm a bit new to this so please forgive my ignorance, but how would I go about pulling the fix into my Office365 CLI version?

waldekmastykarz commented 5 years ago

No problem, thanks for asking. The following steps should do it:

  1. Uninstall the version of the CLI that you have installed globally
  2. Go to https://github.com/waldekmastykarz/office365-cli and clone my fork
  3. Pull the fix-numbers-unknownargs branch
  4. Run npm i to restore dependencies
  5. Run npm run build to build the project
  6. Run npm link to register the local version of the CLI globally
  7. You can now test the fix, just as if you would be using the CLI normally
  8. When you're done, run npm unlink to remove the global registration of the CLI
  9. You can now install the regular build of the CLI again

Hope this helps, and please, don't hesitate to reach out if you're stuck at any of the steps

waldekmastykarz commented 5 years ago

Fix available in the latest beta