Skip to content

Spec for PMUI package source mapping#11922

Merged
donnie-msft merged 19 commits intodevfrom
dev-mcnallyella-PMUIPackageSourceMappingSpec
Dec 5, 2022
Merged

Spec for PMUI package source mapping#11922
donnie-msft merged 19 commits intodevfrom
dev-mcnallyella-PMUIPackageSourceMappingSpec

Conversation

@mcnallyella
Copy link
Contributor

@mcnallyella mcnallyella commented Jun 27, 2022

Fixes: #11378


* Remove: Lets user remove selected package source mapping from the config.

* Clear: Lets user clear all package source mappings from the current config.
Copy link
Contributor

@JonDouglas JonDouglas Jul 8, 2022

Choose a reason for hiding this comment

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

Do we have any worry that a user will hit this button out of curiosity and be burdened to lost work? Should we rather allow a mass selection and keep the remove metaphor instead? On re-thinking this detail, I think the design was mostly parity of similar screens. Do we want that here though?


## Drawbacks

Currently, mappings to sources are not deleted in the config file when the source they are mapped to is deleted. It is complicated to remove this code because there are multiple different config files this mapping could be in.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's also something we shouldn't worry about too much. Sometimes sources can get lost with bugs, bad check-ins, etc. We should preserve their source of truth. This would require users to refactor their code to remove old mappings. Our tooling I don't think can be smart enough to do this on behalf of people as it may lead to undesired behavior.

Copy link
Contributor

@JonDouglas JonDouglas left a comment

Choose a reason for hiding this comment

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

Great specs! Added some comments to consider.


#### Error Messages

There are a few errors the user could make while trying to install or update a package with a mapping that would make restore fail. I will address those errors with a popup telling the user what did not work. First, there could be issues with the transitive packages. Maybe there are transitive packages that are already mapped to a different source or the source that the user is trying to map a package to does not support some of the transitive packages. In either case, a popup would appear telling the user that the package source mapping could not be made. The popup would have a grid with a column showing the top level package and all of the transitive packages, a column showing if each package already has a mapping (if so what source it is mapped to previously), and whether the source supports the package. This error message will look like:
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating a new pop-up window for source mapping errors, can we show an error entry in VS Error Window?

@mcnallyella mcnallyella marked this pull request as ready for review July 19, 2022 16:05
@mcnallyella mcnallyella requested a review from a team as a code owner July 19, 2022 16:05
@mcnallyella mcnallyella changed the title [draft] spec for PMUI package source mapping Spec for PMUI package source mapping Jul 19, 2022
|---|---|---|---|
|`Add additional mapping to source` | Package source mapping is enabled. New mapping is created. Package is now mapped to only the new source| New mapping is created. Package is now mapped to the new source in addition to previous mappings | New mapping is created. Package is now mapped to only the new source.|
|`Use only existing mappings` | Not possible since this button will be greyed out (since there are no existing mappings)| No new mappings are made. Package only has previous mappings| Not possible since this button will be greyed out (since there are no existing mappings). User must select `Add additional mapping to source` to install. |
|Neither is checked| Package Source Mapping is not enabled. Restore works as normal | Not possible since `Use only existing mappings` will be chosen by default once package source mapping is enabled| Not possible since `Use only existing mappings` will be chosen by default once package source mapping is enabled (unless there are no existing mappings in which case the user must select the other option to `Install`)|
Copy link
Member

Choose a reason for hiding this comment

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

We should probably just hide it if the package source mapping is not enabled.


On the details pane there will be two `RadioButtons` that allow the user to choose if they want to add a mapping to the package from the selected source when they `Install`/`Update` or if they only want to use existing mappings. These `RadioButtons` will always be shown even if package source mapping is not enabled, but neither button will have to be selected to install. Package source mapping can be enabled by the user choosing the `RadioButton` that adds a new mapping. Once package source mapping is enabled, the user must select one of the two `RadioButtons` to install. The `Use only existing mappings` option will be selected by default once package source mapping is enabled. The existing mappings will not be shown, but there will be a link to the package source mapping options page so the user can view the mappings. If package source mapping is enabled, but there are no existing mappings, then the `Use only existing mappings` button will be disabled. A label will also appear under the `Use only existing mappings` button if it is disabled that says there are no existing mappings. The link will be a settings icon next to the `RadioButton` (similar to the settings button next to the sources dropdown in the top right of the PMUI). The details pane will look like:

![PMUI 1](../../meta/resources/PackageSourceMapping/PMUI_details_pane.png)
Copy link
Member

Choose a reason for hiding this comment

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

What happens when the source selected is all?

What does add mapping do?
Which source would it add mapping for?

Copy link
Contributor

@dominoFire dominoFire left a comment

Choose a reason for hiding this comment

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

Looks good. Only one question that can be answered later.


### Functional Explanation

On the details pane there will be two `RadioButtons` that allow the user to choose if they want to add a mapping to the package from the selected source when they `Install`/`Update` or if they only want to use existing mappings. These `RadioButtons` will always be shown even if package source mapping is not enabled, but neither button will have to be selected to install. Package source mapping can be enabled by the user choosing the `RadioButton` that adds a new mapping. Once package source mapping is enabled, the user must select one of the two `RadioButtons` to install. The `Use only existing mappings` option will be selected by default once package source mapping is enabled. The existing mappings will not be shown, but there will be a link to the package source mapping options page so the user can view the mappings. If package source mapping is enabled, but there are no existing mappings, then the `Use only existing mappings` button will be disabled. A label will also appear under the `Use only existing mappings` button if it is disabled that says there are no existing mappings. The link will be a settings icon next to the `RadioButton` (similar to the settings button next to the sources dropdown in the top right of the PMUI). The details pane will look like:
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the source mapping ratio button group be visible for packages.config projects?

What happens in Project PM UI ?

I am assuming this feature will only work with PackageReference projects in Promect PM UI.

@kartheekp-ms
Copy link
Contributor

kartheekp-ms commented Aug 23, 2022

Team Triage: Assigning this PR @donnie-msft to drive this work to completion.

@ghost ghost added the Status:No recent activity No recent activity. label Sep 22, 2022
@ghost
Copy link

ghost commented Sep 22, 2022

This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 15 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

@ghost ghost removed the Status:No recent activity No recent activity. label Oct 5, 2022
@donnie-msft donnie-msft removed their assignment Oct 5, 2022
@ghost ghost added the Status:No recent activity No recent activity. label Nov 5, 2022
@ghost
Copy link

ghost commented Nov 5, 2022

This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 15 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

@ghost ghost closed this Nov 20, 2022
@donnie-msft donnie-msft reopened this Dec 5, 2022
@ghost ghost removed the Status:No recent activity No recent activity. label Dec 5, 2022
@donnie-msft
Copy link
Contributor

Merging and will start a clean PR modify it with recent decisions: #11378

@donnie-msft donnie-msft merged commit 03bc069 into dev Dec 5, 2022
@donnie-msft donnie-msft deleted the dev-mcnallyella-PMUIPackageSourceMappingSpec branch December 5, 2022 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Design Package Source mapping package installation experiences in VS PM UI

6 participants