KS-309: use jobProposalSpec id for WF auto approval (#13455)

* use spec id for WF auto approval

* fix tests

.changeset/popular-cycles-divide.md

@@ -0,0 +1,5 @@
+---
+"chainlink": minor
+---
+
+#bugfix use correct internal id in workflow auto-approval

core/services/feeds/service.go

@@ -579,6 +579,8 @@ func (s *service) ProposeJob(ctx context.Context, args *ProposeJobArgs) (int64,
 	)
 
 	var id int64
+	// we need the specID to auto-approve workflow specs
+	var specID int64
 	err = s.orm.Transact(ctx, func(tx ORM) error {
 		var txerr error
 
@@ -598,7 +600,7 @@ func (s *service) ProposeJob(ctx context.Context, args *ProposeJobArgs) (int64,
 		}
 
 		// Create the spec version
-		_, txerr = tx.CreateSpec(ctx, JobProposalSpec{
+		specID, txerr = tx.CreateSpec(ctx, JobProposalSpec{
 			Definition:    args.Spec,
 			Status:        SpecStatusPending,
 			Version:       args.Version,
@@ -616,7 +618,7 @@ func (s *service) ProposeJob(ctx context.Context, args *ProposeJobArgs) (int64,
 	// auto approve workflow specs
 	if isWFSpec(logger, args.Spec) {
 		promWorkflowRequests.Inc()
-		err = s.ApproveSpec(ctx, id, true)
+		err = s.ApproveSpec(ctx, specID, true)
 		if err != nil {
 			promWorkflowFailures.Inc()
 			logger.Errorw("Failed to auto approve workflow spec", "id", id, "err", err)

core/services/feeds/service_test.go

@@ -678,10 +678,11 @@ targets:
     inputs: 
       consensus_output: $(a-consensus.outputs)
 `
-		wfSpec       = testspecs.GenerateWorkflowSpec(wfID, wfOwner, wfName, specYaml).Toml()
-		proposalIDWF = int64(11)
-		remoteUUIDWF = uuid.New()
-		argsWF       = &feeds.ProposeJobArgs{
+		wfSpec              = testspecs.GenerateWorkflowSpec(wfID, wfOwner, wfName, specYaml).Toml()
+		proposalIDWF        = int64(11)
+		jobProposalSpecIdWF = int64(101)
+		remoteUUIDWF        = uuid.New()
+		argsWF              = &feeds.ProposeJobArgs{
 			FeedsManagerID: 1,
 			RemoteUUID:     remoteUUIDWF,
 			Spec:           wfSpec,
@@ -713,7 +714,7 @@ targets:
 			before: func(svc *TestService) {
 				svc.orm.On("GetJobProposalByRemoteUUID", mock.Anything, argsWF.RemoteUUID).Return(new(feeds.JobProposal), sql.ErrNoRows)
 				svc.orm.On("UpsertJobProposal", mock.Anything, &jpWF).Return(proposalIDWF, nil)
-				svc.orm.On("CreateSpec", mock.Anything, proposalSpecWF).Return(int64(100), nil)
+				svc.orm.On("CreateSpec", mock.Anything, proposalSpecWF).Return(jobProposalSpecIdWF, nil)
 				svc.orm.On("CountJobProposalsByStatus", mock.Anything).Return(&feeds.JobProposalCounts{}, nil)
 				transactCall := svc.orm.On("Transact", mock.Anything, mock.Anything)
 				transactCall.Run(func(args mock.Arguments) {
@@ -722,7 +723,7 @@ targets:
 				})
 				// Auto approve is really a call to ApproveJobProposal and so we have to mock that as well
 				svc.connMgr.On("GetClient", argsWF.FeedsManagerID).Return(svc.fmsClient, nil)
-				svc.orm.EXPECT().GetSpec(mock.Anything, proposalIDWF).Return(&proposalSpecWF, nil)
+				svc.orm.EXPECT().GetSpec(mock.Anything, jobProposalSpecIdWF).Return(&proposalSpecWF, nil)
 				svc.orm.EXPECT().GetJobProposal(mock.Anything, proposalSpecWF.JobProposalID).Return(&jpWF, nil)
 				svc.jobORM.On("AssertBridgesExist", mock.Anything, mock.IsType(pipeline.Pipeline{})).Return(nil)
 
@@ -741,7 +742,7 @@ targets:
 					Return(nil)
 				svc.orm.On("ApproveSpec",
 					mock.Anything,
-					proposalSpecWF.JobProposalID,
+					jobProposalSpecIdWF,
 					mock.IsType(uuid.UUID{}),
 				).Return(nil)
 				svc.fmsClient.On("ApprovedJob",
@@ -755,12 +756,13 @@ targets:
 			args:   argsWF,
 			wantID: proposalIDWF,
 		},
+
 		{
 			name: "Auto approve WF spec: error creating job",
 			before: func(svc *TestService) {
 				svc.orm.On("GetJobProposalByRemoteUUID", mock.Anything, argsWF.RemoteUUID).Return(new(feeds.JobProposal), sql.ErrNoRows)
 				svc.orm.On("UpsertJobProposal", mock.Anything, &jpWF).Return(proposalIDWF, nil)
-				svc.orm.On("CreateSpec", mock.Anything, proposalSpecWF).Return(int64(100), nil)
+				svc.orm.On("CreateSpec", mock.Anything, proposalSpecWF).Return(jobProposalSpecIdWF, nil)
 				//			svc.orm.On("CountJobProposalsByStatus", mock.Anything).Return(&feeds.JobProposalCounts{}, nil)
 				transactCall := svc.orm.On("Transact", mock.Anything, mock.Anything)
 				transactCall.Run(func(args mock.Arguments) {
@@ -769,7 +771,7 @@ targets:
 				})
 				// Auto approve is really a call to ApproveJobProposal and so we have to mock that as well
 				svc.connMgr.On("GetClient", argsWF.FeedsManagerID).Return(svc.fmsClient, nil)
-				svc.orm.EXPECT().GetSpec(mock.Anything, proposalIDWF).Return(&proposalSpecWF, nil)
+				svc.orm.EXPECT().GetSpec(mock.Anything, jobProposalSpecIdWF).Return(&proposalSpecWF, nil)
 				svc.orm.EXPECT().GetJobProposal(mock.Anything, proposalSpecWF.JobProposalID).Return(&jpWF, nil)
 				svc.jobORM.On("AssertBridgesExist", mock.Anything, mock.IsType(pipeline.Pipeline{})).Return(nil)