diff --git a/.gitignore b/.gitignore index eedcf4e4..f0344313 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ .vscode/ +ptd/ .gitattributes -*.code-workspace \ No newline at end of file +*.code-workspace +.claude/worktrees/ \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fe391c9..3dd5aba0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.18.0] - Unreleased + +### Added +- Record map classes and their generated data classes are now automatically added to source control (#955) + ## [2.17.1] - 2026-08-18 ### Fixed diff --git a/cls/SourceControl/Git/Extension.cls b/cls/SourceControl/Git/Extension.cls index 4b65659b..3796cc15 100644 --- a/cls/SourceControl/Git/Extension.cls +++ b/cls/SourceControl/Git/Extension.cls @@ -64,10 +64,10 @@ Method UserAction(Type As %Integer, Name As %String, InternalName As %String, Se do ##class(SourceControl.Git.Change).GetUncommitted(filename,.tAction) do ..GetStatus(.InternalName, .isInSourceControl, .isEditable,.isCheckedOut,.userCheckedOut) - // Deal with Business Processes and Rules - // Note: Business Processes and Rules do not have a 'new document' User Action, and thus must be added like this + // certain portal UIs create and compile classes without going through the 'new document' user + // action hooks. we add them explicitly here instead. if (('isInSourceControl)) { - do ..CheckBusinessProcessesAndRules(InternalName) + do ..CheckClassesToAddExplicitly(InternalName) } if '$data(tAction) { @@ -404,9 +404,11 @@ Method OnAfterSave(InternalName As %String, Object As %RegisteredObject = {$$$NU if '##class(SourceControl.Git.Change).IsUncommitted(filename) { $$$ThrowOnError(##class(SourceControl.Git.Change).SetUncommitted(filename, "edit", InternalName, $username, "", 1, "", "", 0)) } + // If this is a RecordMap, keep its generated data class under source control too. + $$$ThrowOnError(##class(SourceControl.Git.Util.RecordMap).AddRelatedClasses(InternalName, 1)) } } else { - do ..CheckBusinessProcessesAndRules(InternalName) + do ..CheckClassesToAddExplicitly(InternalName) } } } catch e { @@ -593,17 +595,18 @@ Method CheckCommitterIdentity(Settings As SourceControl.Git.Settings, ByRef Acti return 0 } -/// Deal with Business Processes and Rules -Method CheckBusinessProcessesAndRules(InternalName As %String) As %Status +/// Add classes whose editors don't respect the 'new document' User Action, so we can't rely on the +/// normal add-on-create flow for them. +Method CheckClassesToAddExplicitly(InternalName As %String) As %Status { - // Note: Business Processes and Rules are not added through normal user action processes because of upstream hook issues, - // so we have to add them like this + // Business Processes, Business Rules, and Record Maps handled here if (##class(SourceControl.Git.Utils).Type(InternalName) = "cls") { set name = $piece(InternalName,".CLS",1) if '##class(%Dictionary.CompiledClass).%ExistsId(name) quit if '$system.CLS.IsMthd(name, "%Extends") quit try { - if ($classmethod(name,"%Extends","Ens.BusinessProcess") || $classmethod(name,"%Extends","Ens.Rule.Definition")) { + if ($classmethod(name,"%Extends","Ens.BusinessProcess") || $classmethod(name,"%Extends","Ens.Rule.Definition") + || $classmethod(name,"%Extends","EnsLib.RecordMap.RecordMap")) { do ..AddToSourceControl(InternalName) } } catch err { diff --git a/cls/SourceControl/Git/Util/RecordMap.cls b/cls/SourceControl/Git/Util/RecordMap.cls new file mode 100644 index 00000000..7e7824c9 --- /dev/null +++ b/cls/SourceControl/Git/Util/RecordMap.cls @@ -0,0 +1,72 @@ +Include (%occStatus, %occErrors, SourceControl.Git) + +/// Utilities for keeping the classes generated by a RecordMap (its data/target class and any batch +/// class) under source control alongside the RecordMap itself. +Class SourceControl.Git.Util.RecordMap +{ + +/// Ensure the generated data/target class(es) of a RecordMap are under source control. For each +/// related class reported by GetRelatedClasses that is not already tracked, add it. +/// Used both when a RecordMap is first added and when it is saved/compiled, so its data class always +/// comes along. Does nothing for non-RecordMap items. +ClassMethod AddRelatedClasses(InternalName As %String, refreshUncommitted As %Boolean = 1) As %Status +{ + set ec = $$$OK + do ..GetRelatedClasses(InternalName, .relatedClasses) + set added = 0 + set relatedKey = $order(relatedClasses("")) + while (relatedKey '= "") { + if '##class(SourceControl.Git.Utils).IsInSourceControl(relatedKey) { + set sc = ##class(SourceControl.Git.Utils).AddToSourceControl(relatedKey, 0) + set added = 1 + if 'sc { + set ec = $$$ADDSC(ec, sc) + } + } + set relatedKey = $order(relatedClasses(relatedKey)) + } + if added && refreshUncommitted { + do ##class(SourceControl.Git.Change).RefreshUncommitted(,,,1) + } + quit ec +} + +/// For a RecordMap class (one that extends EnsLib.RecordMap.RecordMap), report the generated +/// data/target class(es) that should be source-controlled alongside it. These classes are not +/// regenerated by recompiling the RecordMap, so they must be tracked independently. +/// related is returned subscripted by the internal name of each related class. +ClassMethod GetRelatedClasses(InternalName As %String, Output related) As %Status +{ + kill related + set sc = $$$OK + try { + if (##class(SourceControl.Git.Utils).Type(.InternalName) '= "cls") quit + if '##class(%Library.EnsembleMgr).IsEnsembleNamespace() quit + set className = $piece(##class(SourceControl.Git.Utils).NormalizeInternalName(InternalName), ".CLS", 1) + if '##class(%Dictionary.CompiledClass).%ExistsId(className) quit + if '$system.CLS.IsMthd(className, "%Extends") quit + if '$classmethod(className, "%Extends", "EnsLib.RecordMap.RecordMap") quit + + // GetGeneratedClasses reports the generated classes (target, batch, etc.) plus the RecordMap + // itself; the RecordMap is excluded below. It is unioned with the OBJECTNAME parameter (the + // target class), which acts as a fallback for when GetGeneratedClasses is unreliable. + // GetGeneratedClasses kills its output array, so the OBJECTNAME set must stay after the call. + kill candidates + do $classmethod(className, "GetGeneratedClasses", .candidates) + set objectName = $parameter(className, "OBJECTNAME") + if (objectName '= "") set candidates(objectName) = "" + + set candidate = $order(candidates("")) + while (candidate '= "") { + if (candidate '= className) && $$$defClassDefined(candidate) { + set related(candidate _ ".CLS") = "" + } + set candidate = $order(candidates(candidate)) + } + } catch e { + set sc = e.AsStatus() + } + quit sc +} + +} diff --git a/cls/SourceControl/Git/Utils.cls b/cls/SourceControl/Git/Utils.cls index 5a5dd68d..240cfbad 100644 --- a/cls/SourceControl/Git/Utils.cls +++ b/cls/SourceControl/Git/Utils.cls @@ -878,6 +878,13 @@ ClassMethod AddToSourceControl(InternalName As %String, refreshUncommitted As %B do ..PrintStreams(outStream, errStream) } + + // If this is a RecordMap class, also add its generated data/target class(es), which the + // developer would otherwise have to remember to add manually. AddRelatedClasses refreshes + // the uncommitted list itself below, so skip the per-item refresh here. + if (type = "cls") { + set ec = $$$ADDSC(ec, ##class(SourceControl.Git.Util.RecordMap).AddRelatedClasses(item, 0)) + } } if refreshUncommitted { do ##class(SourceControl.Git.Change).RefreshUncommitted(,,,1) diff --git a/module.xml b/module.xml index 66e17898..46f06b83 100644 --- a/module.xml +++ b/module.xml @@ -3,7 +3,7 @@ git-source-control - 2.17.1 + 2.18.0 Server-side source control extension for use of Git on InterSystems platforms git source control studio vscode module diff --git a/test/UnitTest/SourceControl/Git/RecordMap.cls b/test/UnitTest/SourceControl/Git/RecordMap.cls new file mode 100644 index 00000000..1f14523c --- /dev/null +++ b/test/UnitTest/SourceControl/Git/RecordMap.cls @@ -0,0 +1,200 @@ +Import SourceControl.Git + +/// Tests for automatically source-controlling the data/target classes generated by a RecordMap. +Class UnitTest.SourceControl.Git.RecordMap Extends UnitTest.SourceControl.Git.AbstractTest +{ + +Parameter FIXTUREPACKAGE = "UnitTest.SourceControl.Git.RecordMapFixture"; + +Method %OnClose() As %Status [ Private, ServerOnly = 1 ] +{ + do $system.OBJ.DeletePackage(..#FIXTUREPACKAGE, "-d") + quit ##super() +} + +/// Build a real, compiled RecordMap whose target/record class is generated as a separate class, +/// mirroring what the RecordMap editor produces. Returns the record map class name. +ClassMethod CreateRecordMap(pMap As %String, pTarget As %String) As %Status +{ + do $system.OBJ.Delete(pMap, "-d") + do $system.OBJ.Delete(pTarget, "-d") + set model = ##class(EnsLib.RecordMap.Model.Record).%New() + set model.name = pMap + set model.type = "delimited" + set model.targetClassname = pTarget + set model.recordTerminator = $char(13, 10) + do model.Separators.Insert(",") + set field = ##class(EnsLib.RecordMap.Model.Field).%New() + set field.name = "Field1" + do model.Contents.Insert(field) + set sc = model.SaveToClass() + if $$$ISERR(sc) quit sc + set target = pTarget + quit ##class(EnsLib.RecordMap.Generator).GenerateObject(pMap, .target, , , 2) +} + +/// Build a RecordMap associated with a (simple) Batch class, mirroring what the RecordMap editor +/// produces when a batch class is configured. The batch class must exist before generation. +ClassMethod CreateBatchRecordMap(pMap As %String, pTarget As %String, pBatch As %String) As %Status +{ + do $system.OBJ.Delete(pMap, "-d") + do $system.OBJ.Delete(pTarget, "-d") + do $system.OBJ.Delete(pBatch, "-d") + set batchDef = ##class(%Dictionary.ClassDefinition).%New() + set batchDef.Name = pBatch + set batchDef.Super = "EnsLib.RecordMap.Batch" + set param = ##class(%Dictionary.ParameterDefinition).%New() + set param.Name = "RECORDMAPGENERATED" + set param.Default = 1 + set param.parent = batchDef + set sc = batchDef.%Save() + if $$$ISERR(sc) quit sc + set sc = $system.OBJ.Compile(pBatch, "ck") + if $$$ISERR(sc) quit sc + set model = ##class(EnsLib.RecordMap.Model.Record).%New() + set model.name = pMap + set model.type = "delimited" + set model.targetClassname = pTarget + set model.batchClass = pBatch + set model.recordTerminator = $char(13, 10) + do model.Separators.Insert(",") + set field = ##class(EnsLib.RecordMap.Model.Field).%New() + set field.name = "Field1" + do model.Contents.Insert(field) + set sc = model.SaveToClass() + if $$$ISERR(sc) quit sc + set target = pTarget + quit ##class(EnsLib.RecordMap.Generator).GenerateObject(pMap, .target, , , 2) +} + +/// Build a trivial compiled class that is not a RecordMap, for exercising the non-RecordMap path. +ClassMethod CreatePlainClass(pClass As %String) As %Status +{ + do $system.OBJ.Delete(pClass, "-d") + set def = ##class(%Dictionary.ClassDefinition).%New() + set def.Name = pClass + set def.Super = "%Persistent" + set sc = def.%Save() + if $$$ISERR(sc) quit sc + quit $system.OBJ.Compile(pClass, "ck") +} + +/// The resolver should report the generated target/data class as related to the RecordMap, +/// and must not report the RecordMap class itself. +Method TestResolverIncludesTargetClass() +{ + set map = ..#FIXTUREPACKAGE_".Map" + set target = ..#FIXTUREPACKAGE_".Record" + $$$ThrowOnError(..CreateRecordMap(map, target)) + + do $$$AssertStatusOK(##class(SourceControl.Git.Util.RecordMap).GetRelatedClasses(map_".CLS", .related)) + do $$$AssertTrue($data(related(target_".CLS")), "target data class should be reported as related to the RecordMap") + do $$$AssertNotTrue($data(related(map_".CLS")), "the RecordMap class itself should not be reported as related") +} + +/// Adding a RecordMap class to source control should automatically add its generated data class. +Method TestAddRecordMapAddsTargetClass() +{ + set map = ..#FIXTUREPACKAGE_".Map" + set target = ..#FIXTUREPACKAGE_".Record" + $$$ThrowOnError(..CreateRecordMap(map, target)) + + do $$$AssertNotTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target should not be in source control before the RecordMap is added") + $$$ThrowOnError(##class(SourceControl.Git.Utils).AddToSourceControl(map_".CLS")) + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(map_".CLS"), "RecordMap class should be in source control") + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target data class should be added to source control automatically") +} + +/// A RecordMap that is already tracked but whose generated data class is not (e.g. the map was +/// added before the data class existed) should have its data class pulled back under source control +/// when the map is saved/compiled. OnAfterSave delegates this to AddRelatedClasses, which is +/// exercised directly here to avoid depending on a fully configured git working tree. +Method TestSaveTrackedRecordMapAddsUntrackedTarget() +{ + set map = ..#FIXTUREPACKAGE_".Map" + set target = ..#FIXTUREPACKAGE_".Record" + $$$ThrowOnError(..CreateRecordMap(map, target)) + + // Precondition: the RecordMap is tracked but its generated data class is not. + $$$ThrowOnError(##class(SourceControl.Git.Utils).AddToSourceControl(map_".CLS", 0)) + $$$ThrowOnError(##class(SourceControl.Git.Utils).RemoveFromSourceControl(target_".CLS", 0)) + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(map_".CLS"), "RecordMap should be tracked before save") + do $$$AssertNotTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target should not be tracked before save") + + // Saving the RecordMap should bring its data class back under source control. + $$$ThrowOnError(##class(SourceControl.Git.Util.RecordMap).AddRelatedClasses(map_".CLS", 0)) + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "saving the RecordMap should re-add its generated data class to source control") +} + +/// For a RecordMap that has an associated Batch class, the resolver should report both the target +/// data class and the batch class as related. The batch class is discovered via GetGeneratedClasses +/// rather than the OBJECTNAME parameter. +Method TestResolverIncludesBatchClass() +{ + set map = ..#FIXTUREPACKAGE_".BMap" + set target = ..#FIXTUREPACKAGE_".BRecord" + set batch = ..#FIXTUREPACKAGE_".BBatch" + $$$ThrowOnError(..CreateBatchRecordMap(map, target, batch)) + + do $$$AssertStatusOK(##class(SourceControl.Git.Util.RecordMap).GetRelatedClasses(map_".CLS", .related)) + do $$$AssertTrue($data(related(target_".CLS")), "target data class should be reported as related") + do $$$AssertTrue($data(related(batch_".CLS")), "batch class should be reported as related") + do $$$AssertNotTrue($data(related(map_".CLS")), "the RecordMap class itself should not be reported as related") +} + +/// A class that does not extend EnsLib.RecordMap.RecordMap has no related classes, and running the +/// resolver/adder against it is a no-op that adds nothing to source control. +Method TestNonRecordMapClassHasNoRelated() +{ + set plain = ..#FIXTUREPACKAGE_".Plain" + $$$ThrowOnError(..CreatePlainClass(plain)) + + do $$$AssertStatusOK(##class(SourceControl.Git.Util.RecordMap).GetRelatedClasses(plain_".CLS", .related)) + do $$$AssertEquals($order(related("")), "", "a non-RecordMap class should report no related classes") + + do $$$AssertStatusOK(##class(SourceControl.Git.Util.RecordMap).AddRelatedClasses(plain_".CLS", 0)) + do $$$AssertNotTrue(##class(SourceControl.Git.Utils).IsInSourceControl(plain_".CLS"), "a non-RecordMap class should not be added to source control") +} + +/// A newly compiled RecordMap that isn't yet tracked (e.g. just generated via the RecordMap wizard, +/// which never issues a "new document" add) should be picked up by CheckClassesToAddExplicitly, the +/// same fallback that already exists for classes created by other portal UIs like the Business +/// Process/Rule editors. This should also bring along the generated target class via the +/// AddToSourceControl cascade. +Method TestUntrackedRecordMapIsAddedOnCompile() +{ + set map = ..#FIXTUREPACKAGE_".CMap" + set target = ..#FIXTUREPACKAGE_".CRecord" + $$$ThrowOnError(..CreateRecordMap(map, target)) + + do $$$AssertNotTrue(##class(SourceControl.Git.Utils).IsInSourceControl(map_".CLS"), "RecordMap should not be tracked before the compile-time check") + do $$$AssertNotTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target should not be tracked before the compile-time check") + + new %session, %request, %SourceControl + set %session = ##class(%CSP.Session).%New("dummysession") + set %request = ##class(%CSP.Request).%New() + set %request.Data("pageclass",1) = "EnsPortal.dummy" + do ##class(%Studio.SourceControl.Interface).SourceControlCreate() + do %SourceControl.CheckClassesToAddExplicitly(map_".CLS") + + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(map_".CLS"), "RecordMap should be added to source control by the compile-time fallback check") + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target data class should be added automatically alongside the RecordMap") +} + +/// Calling AddRelatedClasses more than once is idempotent: the generated data class is added and +/// remains tracked, and a repeat call succeeds without error. +Method TestAddRelatedClassesIsIdempotent() +{ + set map = ..#FIXTUREPACKAGE_".IMap" + set target = ..#FIXTUREPACKAGE_".IRecord" + $$$ThrowOnError(..CreateRecordMap(map, target)) + + do $$$AssertNotTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target should not be tracked before the first call") + $$$ThrowOnError(##class(SourceControl.Git.Util.RecordMap).AddRelatedClasses(map_".CLS", 0)) + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target should be tracked after the first call") + + do $$$AssertStatusOK(##class(SourceControl.Git.Util.RecordMap).AddRelatedClasses(map_".CLS", 0)) + do $$$AssertTrue(##class(SourceControl.Git.Utils).IsInSourceControl(target_".CLS"), "target should still be tracked after a repeat call, with no error") +} + +}