@f.PartialFor, @f.Partial, @s.PartialFor, @s.Partial

[robdmoore]

Alternative implementation to #113

I wanted to try and understand some of the complexity in #113 and why it's there. There were a few things I had to make simpler, but all in all most of the structure from #113 has been retained.

Would be good to get a fresh pair of eyes on this @MattDavies @Royce

cc @zabulus

[robdmoore]

This view shows all the usages:

@f.Partial(...) / @f.PartialFor(m => m, ...)
@f.PartialFor(m => m.Child, ...)
@s.Partial(...) / @s.PartialFor(m => m, ...)
@s.PartialFor(m => m.Child, ...)

[zabulus]

You thrown away all that stuff with expression rewriting. Good. It was really overcomplicated.
Looks like I'm not very familiar with the expression API.

[robdmoore]

I still needed the Combine method you wrote though.

Did you write that from scratch or pull from stack overflow by the way? I'd like to attribute of if the latter so we can find the original source.

On 5 Oct 2015, at 1:30 am, zabulus [email protected] wrote:

You thrown away all that stuff with expression rewriting. Good. It was really overcomplicated.
Looks like I'm not very familiar with the expression API.

—
Reply to this email directly or view it on GitHub.

[zabulus]

It was from stackoverflow. See this questions:
http://stackoverflow.com/questions/10898800/combine-two-linq-expressions-to-inject-navigation-property
http://stackoverflow.com/questions/21776276/combing-two-linq-expressions

[robdmoore]

I've added test coverage now.

Had to do it in the Acceptance Tests project.

Only thing I haven't put in is the PartialForList command that @zabulus implemented.

I'm inclined to get this in and leave that as an exercise for someone else (@zabulus or otherwise) since I don't think it's essential.

@MattDavies can you cast your eye over and merge if happy?

[robdmoore]

FYI This drops the code coverage stats heaps, but the code is actually covered due to the Acceptance Test

[robdmoore]

This is from a previous PR, but I noticed that it was missing so added in.

I suspect we can release v2 after this PR is merged in. Thoughts @MattDavies?

[robdmoore]

It would be nice to get my other WIP PR merged as well, but not sure when I'll have time to finish it.

[robdmoore]

I've added a bunch of tweaks I called out.

Only thing left is to add some doco to the wiki after it's merged

[robdmoore]

wiki entry done

[zabulus]

I think also need to add submit test, and check that POST binding works as expected.

[robdmoore]

Makes sense.

Let's do it in a separate PR though. I'm happy enough that it will postback ok in reviewing the approval test.

On 12 Oct 2015, at 5:28 pm, zabulus [email protected] wrote:

In ChameleonForms.AcceptanceTests/PartialForTests.cs:

+using System.IO;
+using System.Net.Http;
+using System.Text.RegularExpressions;
+using ApprovalTests.Html;
+using ApprovalTests.Reporters;
+using NUnit.Framework;
+using OpenQA.Selenium;
+using OpenQA.Selenium.Support.UI;
+
+namespace ChameleonForms.AcceptanceTests
+{

• ///

• /// Loading partial views is very difficult to test by unit testing.

• ///

• [UseReporter(typeof(DiffReporter))]

• public class PartialForTests
  I think also need to add submit test, and check that POST binding works as expected.

—
Reply to this email directly or view it on GitHub.