Conversation
| @@ -0,0 +1,13687 @@ | |||
| { | |||
| "name": "simple_dtl_ui", | |||
There was a problem hiding this comment.
Might consider removing this from the repo if it's a generated file
There was a problem hiding this comment.
Other Microsoft repos I've checked have included this file -- might make sense to ensure that users get the exact same set of dependencies when they try out the sample
There was a problem hiding this comment.
Sure, sounds good - if it's common to include it we should just leave it
| RequestId = Activity.Current?.Id ?? HttpContext.TraceIdentifier; | ||
| } | ||
| } | ||
| } No newline at end of file |
| 1. [Follow these instructions to register your application.](https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-register-app) You will want to select the "Single-page application" tile when configuring platform settings. | ||
| 2. In the management panel of your app registration, select *API permissions* > *Add a permission* > *Azure Service Management* | ||
| 3. Select the *user_impersonation* permission | ||
| 4. Select *Add permissions* to complete |
There was a problem hiding this comment.
Double check for locked down AAD tenants if there is an extra step for global aad admins to do... If so, we should add a note for this in the readme
There was a problem hiding this comment.
It seems that, like we noted, "Admin consent required" is set to false in the Portal:

However, when I try logging in via the client, I get this error:

Does this seem like the appropriate article to link to? https://docs.microsoft.com/en-us/azure/active-directory/manage-apps/grant-admin-consent The user will need to figure out who their AAD admins are, though
There was a problem hiding this comment.
We should talk about this in our next meeting - tenant wide consent is a big hammer and there might be a less invasive approach... Thanks!
There was a problem hiding this comment.
Sounds good -- I'll make a note of this so that I don't forget to bring it up!
| 2. Next, navigate to your DevTest Lab in the Azure portal. Copy the **lab name, resource group name, and subscription ID** into the `appsettings.json` file. Once you have deployed the application to Azure, you can also set these values in the portal itself by following the instructions [here](https://docs.microsoft.com/en-us/azure/app-service/configure-common). | ||
|
|
||
| ### Deploy the application to Azure | ||
| Right-click the SimpleDtlUI project in Visual Studio, and select *Publish*. |
There was a problem hiding this comment.
Link to the docs on publishing with Visual Studio would be great. Also can link to docs on deploying this with GitHub Actions, or Azure Devops Pipelines
Maybe add a screenshot of the app in the readme
| "ASPNETCORE_ENVIRONMENT": "Development" | ||
| } | ||
| }, | ||
| "simple_dtl_ui": { |
There was a problem hiding this comment.
This launch profile can probably be removed
| import React, { Component } from 'react'; | ||
| import OwnershipButton from './OwnershipButton'; | ||
|
|
||
| export default class App extends Component { |
There was a problem hiding this comment.
Functional component instead of Class component
…mponents instead of class components
samples/DevTestLabs/SimpleUI/ClientApp/src/VirtualMachineTable.jsx
Outdated
Show resolved
Hide resolved
samples/DevTestLabs/SimpleUI/ClientApp/src/VirtualMachineTable.jsx
Outdated
Show resolved
Hide resolved
| import { useAuthContext } from './Hooks'; | ||
| import { OwnershipButton, VMAction } from './OwnershipButton'; | ||
|
|
||
| const renderOwnershipButton = (virtualMachine, loggedInUser) => { |
There was a problem hiding this comment.
Might be worth breaking this into it's own component and then have the render function to just something like:
(virtualMachine, loggedInuser) =>
There was a problem hiding this comment.
@mharlan Do you mean just including this logic in the existing OwnershipButton component? Or refactoring it into another separate one? Would need to think of a good component name, then! 😄
There was a problem hiding this comment.
Either would work. Not sure if you have other situations where you intend the simple OwnershipButton to be used. This just reads like a function component to me.
There was a problem hiding this comment.
Understood! I'll move this into OwnershipButton since I don't think there's much potential for reusability there

This sample is to accompany the SharePoint + DevTest Labs blog series. The intention is to demonstrate the ease with which users of DevTest Labs can create a simplified user experience independent of the Azure portal.
It includes: