nathanwoulfe / Plumber-2

The workflow solution for Umbraco 8 - 10
13 stars 4 forks source link

Consider splitting customizations from core static assets #103

Closed bachratyg closed 2 years ago

bachratyg commented 3 years ago

Is your feature request related to a problem? Please describe. I'm supposed to drop the license into \App_Plugins\Plumber\, I can add custom translations to \App_Plugins\Plumber\lang\ or add custom email templates under \Views\Partials\WorkflowEmails\. However this all gets wiped out when doing a full clean/rebuild because the ClearPlumberPackageAssets target in Plumber.Backoffice.targets indiscriminately removes everything in those folders.

Describe the solution you'd like Split configurable assets from core immutable assets or handle all local assets gracefully (e.g. no forced overwrite). For Umbraco I don't need to change \umbraco\config\lang\en.xml, I can put customizations in\config\lang\en.user.xml` where it is preserved.

Describe alternatives you've considered Add ExcludeAssets="buildTransitive" to the PackageReference and manage static assets manually/via a custom target. It would still be nicer if there was an opt-out switch or some hook to add explicit exclusions e.g.

<ItemGroup>
    <PlumberPreserveFiles Include="App_Plugins\Plumber\test.lic" />
    <PlumberPreserveFiles Include="Views\Partials\WorkflowEmails\FancyNotificationForTheTopGuy.cshtml" />
</ItemGroup>

Licensed version? Not yet. Planning to real soon.

Additional context We're actually hitting this issue on Umbraco 8.17 that is tweaked to utilize an sdk-style project format (based on https://github.com/dotnet/project-system/issues/2670). In the vanilla umbraco setup using the classic csproj format this is less of an issue since Plumber.Backoffice.targets is completely ignored. This setup is likely not supported but the described problem would be the same for U9. I can share a simplified project skeleton if needed.

nathanwoulfe commented 3 years ago

Hey @bachratyg, this is what happens when I rush to get things out the door for a new Umbraco version 🙄

Would appreciate anything you were able to share!

nathanwoulfe commented 3 years ago

Curious too how custom email templates are working - you should be able to modify the defaults, but I don't think adding custom will work, as Plumber is only aware of the defaults. Custom languages should probably be added to the Umbraco user language files, in /config/lang.

Not sure yet how to deal with the license being wiped on clean/rebuild, without changing the license path, which would be a breaking change for all licensed installs...

bachratyg commented 3 years ago

Curious too how custom email templates are working - you should be able to modify the defaults, but I don't think adding custom will work, as Plumber is only aware of the defaults

I meant this: https://github.com/nathanwoulfe/Plumber-2/blob/master/DOCS.md#localization

Custom languages should probably be added to the Umbraco user language files, in /config/lang.

If I understand correctly moving the translation files/overrides from the plugin folder to /config/lang should work just as well? Then this is no longer an issue.

Not sure yet how to deal with the license being wiped on clean/rebuild, without changing the license path

This should probably be configurable. If there is no config then the original path is in effect. UmbracoForms does the same, see https://our.umbraco.com/Documentation/Add-ons/The-Licensing-model/index-v7#alternative-license-location

Would appreciate anything you were able to share!

This is the basic csproj setup with the clutter removed:

<Project>
  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
  <PropertyGroup>
    <OutputType>Library</OutputType>
    <TargetFramework>net48</TargetFramework>
    <LangVersion>latest</LangVersion>
    <OutputPath>bin\</OutputPath>
    <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
    <AppConfig>Web.config</AppConfig>
    <PublishProfileImported>true</PublishProfileImported>
    <EnableDefaultItems>false</EnableDefaultItems>
    <RestorePackagesWithLockFile>true</RestorePackagesWithLockFile>
  </PropertyGroup>
  <ItemGroup>
    <ProjectCapability Include="DotNetCoreWeb" />
    <ProjectCapability Include="SupportsSystemWeb" />
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" Version="2.0.1" />
    <PackageReference Include="Plumber.Workflow" Version="2.0.1" />
    <PackageReference Include="UmbracoCms" Version="8.17.0" />
  </ItemGroup>
  <ItemGroup>
    <Compile Include="**\*$(DefaultLanguageSourceExtension)" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" />
    <EmbeddedResource Include="**\*.resx" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" />
    <Content Include="**\*" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder);**\*$(DefaultLanguageSourceExtension);**\*.resx;Web.config;Web.*.config;App_Data\**\*;packages.lock.json;$(AppDesignerFolder)\**" />
    <None Include="App_Data\**\*" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder);**\*$(DefaultLanguageSourceExtension);**\*.resx" />
    <None Include="$(AppDesignerFolder)\**" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder);**\*$(DefaultLanguageSourceExtension);**\*.resx" />
    <None Include="Web.config" />
    <None Include="Web.*.config" DependentUpon="Web.config" />
  </ItemGroup>
  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
  <PropertyGroup>
    <VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">15.0</VisualStudioVersion>
    <VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>
  </PropertyGroup>
  <Import Project="$(VSToolsPath)\WebApplications\Microsoft.WebApplication.targets" />
  <!-- copy all Umbraco files before build not before publish so that required files are not needed to be checked in and simple build works in a clean repo-->
  <Target Name="AddUmbracoFilesAsContent" DependsOnTargets="CopyUmbracoFilesToWebRoot" BeforeTargets="BeforeBuild">
    <ItemGroup>
      <Content Include="@(UmbracoFiles->'%(RecursiveDir)%(Filename)%(Extension)')" Exclude="@(Content)" Condition=" '%(RecursiveDir)%(Filename)%(Extension)' != 'Web.config' " />
    </ItemGroup>
  </Target>
  <!-- override Umbraco targets to exclude source code and other content not explicitly added from output -->
  <Target Name="AddUmbracoFilesToOutput">
  </Target>
</Project>

Publish profile:

<Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
    <DeleteExistingFiles>True</DeleteExistingFiles>
    <ExcludeApp_Data>False</ExcludeApp_Data>
    <LaunchSiteAfterPublish>True</LaunchSiteAfterPublish>
    <PublishProvider>FileSystem</PublishProvider>
    <PublishUrl>..\.publish\</PublishUrl>
    <WebPublishMethod>FileSystem</WebPublishMethod>
    <SiteUrlToLaunchAfterPublish />
  </PropertyGroup>
</Project>

And then the solution is built like this:

msbuild /target:Restore,Rebuild /p:Configuration=Release /p:DeployOnBuild=true /p:PublishProfile=FolderProfile.pubxml

Now unfortunately the CopyPlumberPackageAssets target does not get a chance to run before the build fails and the plumber folders are missing.

If I don't DeployOnBuild then all tasks seem to run properly (still wiping extras from the plumber folders in the process).

Changing the PackageReference to this:

<PackageReference Include="Plumber.Workflow" Version="2.0.1" ExcludeAssets="buildTransitive" />

fixes the problem, but then I have to copy content manually and this would also disable any logic (to be added in the future) in the targets file.

bachratyg commented 3 years ago

I believe the main issue with the copy target failing to run is that BeforeTargets="Build" means that the target should run just before the Build task. However the SDK is architected so that the Build task is empty and the actual work is happening in the targets that it depends on (BeforeBuild;CoreBuild;AfterBuild). According to https://docs.microsoft.com/en-us/visualstudio/msbuild/target-build-order the targets run in this order:

so it would be probably safer to use BeforeTargets="BeforeBuild" and BeforeTargets="BeforeClean" so that assets are in place as soon as possible.

nathanwoulfe commented 3 years ago

First priority will be updating where the license is stored - will check the configured license path, then app_plugins. If there's nothing in the former, but exists at the latter, will move to the configured path. Something like that.

nathanwoulfe commented 3 years ago

@bachratyg I've had a fiddle with this and have managed to leave the license/key files and any modified email templates untouched on Build/Clean. Will update again to BeforeBuild/BeforeClean, and check that doesn't break anything.

With those changes, I think the license can stay where it is - if it's not being deleted, it should be fine.

And calling back to the language files, I haven't tried overriding from config/lang, but will have a look at what happens. Umbraco docs say user languages will take precedence over core and package files, which makes sense, I just haven't ever tried it...

nathanwoulfe commented 2 years ago

I believe this has been resolved with changes in Plumber.Backoffice.targets, released in v2.0.2. @bachratyg please let me know if the issue remains.