Fixes 'spo site set' command to handle site logo and thumbnail from another site. Closes #6689

[jhholm]

Closes #6689

[Jwaegebaert]

Thanks for jumping in with a PR to fix the bug! Looks like our coverage tests didn’t pass. We require full coverage, and it seems to have trouble with the new string you added in SpoCommand.ts: 'siteThumbnailUrl'. Could you check that out?

[jhholm]

Thanks for noticing. Should be fixed now. Though I did get the same error with coverage even with this fix initially, but after the fourth time it started magically working. I think it's a user error here, cause I'm not familiar with mocha.

So the change in SpoCommand.ts was to get siteThumbnailUrl param to work like siteLogoUrl param, being automatically processed to an absolute Url. That makes the change in site-set.ts work.

[Jwaegebaert]

Alright, good to know. Looks like updating the spec file clears the coverage, so it should be good for review. Thanks again, we'll aim to review it asap!

[waldekmastykarz]

Let's add a test case that proves that this change works. I'll mark the PR as draft for now, so that we know that we should wait with merging it.

[jhholm]

Is this test case something that you would want me to contribute?

I guess the following test case should cover the change

• Test that a given relative URL is being converted correctly to an absolute URL
• Test that an absolute URL is converted back to a relative URL correctly for the REST call

[milanholemans]

What @waldekmastykarz means is that you should add a test to the site-set.spec.ts file that tests if the command is still working fine when specifying a site logo or thumbnail URL that is stored on another site.

[jhholm]

Added 4 additional test cases that fail successfully on the current main branch and do not in this pull request.

[jhholm]

Just making sure that I set this for review correctly, or is there some other input required?

[milanholemans]

Just making sure that I set this for review correctly, or is there some other input required?

You're totally good. We're just a little short in time lately, sorry for that. We'll try to do another review of your PR soon.

[waldekmastykarz]

That's on me @jhholm, sorry. I'll get to it asap. I appreciate your patience with us.

[Adam-it]

checked all 3 scenarios

Works like a charm 🪄
LGMT 👍

[Adam-it]

Ready to merge 🚀

[Adam-it]

Merged manually.
Thank you for your awesome work 👏. You Rock 🤩