Move metric creation (Fixes #5079)

[nlBayside]

Moves metric creation to occur sooner, so that the attribute values have something to be added to in the database.

Notice

We cannot guarantee that your pull request will be accepted. There are many factors involved. We take into consideration whether or not the Rock system you run is a standard, main-line build. If it is not, there is a lower chance we will accept your request (since it may impact a part of the system you don't regularly use). Features that would be used by less than 80% of Rock organizations, or ones that don't match the goals of Rock, are other important factors.

Proposed Changes

Move Lines 507-513 further up in the MetricDetail Block code, so that the attribute values are assigned a proper EntityId on creation.

Fixes: #5079

Types of changes

What types of changes does your code introduce to Rock? Put an x in the boxes that apply

• [X] Bugfix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality, which has been approved by the core team)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• [X] This is a single-commit PR. (If not, please squash your commit and re-submit it.)
• [X] I verified my PR does not include whitespace/formatting changes -- because if it does it will be closed without merging.
• [X] I have read the Contributing to Rock doc
• [X] By contributing code, I agree to license my contribution under the Rock Community License Agreement
• [ ] Unit tests pass locally with my changes
• [ ] I have added any required unit tests or integration tests that prove my fix is effective or that my feature works
• [ ] I have included updated language for the Rock Documentation (if appropriate)

Documentation

N/A

Migrations

N/A

[nairdo]

Hi @nlBayside , I looked at your change and it might be better if this ...GetEditValues( metric ) and its SaveAttributeValues() was moved to after the if ( metric.Id == 0 ) ... metricService.Add( metric ) block instead of moving that save higher like you did. So more like this:

    if ( metric.Id == 0 )
    {
        metricService.Add( metric );

        // save to make sure we have a metricId
        rockContext.SaveChanges();
    }

    // **Now we can safely save the attribute values**

    avcEditAttributeValues.GetEditValues( metric );

    // only save if everything saves:
    rockContext.WrapTransaction( () =>
    {
        rockContext.SaveChanges();
        metric.SaveAttributeValues();
    } );

I know the it's been like this since the b41ccbc commit that added the feature, but looking at that commit again I see there is some validation that occurs before the scheduleService.Add( schedule ); and its rockContext.SaveChanges(); so it would be best to only save the metric after it passes validation.

Let me know if you're not comfortable adjusting / squashing your commit and we can do it over here (but then you would not get the Github merge credit.)

[nlBayside]

Yes, that was a concern I had after creating the commit. I can definitely make adjustments to the change reflecting what you suggested. I'll get that done when I have a chance too soon. Thank you!

[nlBayside]

Hello! I submitted this update to the PR almost a week ago and I'm not sure what the next steps are for Rock. I did notice this page in the Github wiki: Contributing to Rock. It sounds like there's a CLA that someone outside of Spark Development would need to sign. If that's the case still, I'm more than happy to cooperate with your team's internal processes! I haven't filled out the form previously. Thank you!

[nairdo]

Thanks @nlBayside We typically get to pull requests depending on our workload. As far as signing a the Spark Contributor Agreement, that's always a good idea --but submitting a Pull Request here means you've agreed to the terms. (~Since that secretary @ sparkdevnetwork.org email does not appear to be working at the moment, you can email it to nick @ spark... once you sign it.~)

Lastly, are you open to including your name on your messages here? (Since your profile is a bit sparse it would be nice to know who we're talking with -- although I'm pretty sure your name is Noah, right?)