Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow ignores on reverse map paths #2162

Merged
merged 2 commits into from Jun 26, 2017
Merged

Allow ignores on reverse map paths #2162

merged 2 commits into from Jun 26, 2017

Conversation

jbogard
Copy link
Member

@jbogard jbogard commented Jun 26, 2017 •
edited

Should really have done this with 6.1.0, whoops.

Allow you to ignore a path:

cfg =>
    {
		cfg.CreateMap<TimesheetModel, TimesheetViewModel>()
			.ForMember(d=>d.Contact, o=>o.MapFrom(s=>s.ContactNavigation.Id))
			.ReverseMap()
                        .ForPath(s => s.ContactNavigation.Id, opt => opt.Ignore());
    }

@jbogard jbogard added this to the 6.1.1 milestone Jun 26, 2017
@jbogard
Copy link
Member Author

jbogard commented Jun 26, 2017

Closes #2161

@jbogard jbogard requested a review from lbargaoanu June 26, 2017 15:35
@jbogard
Copy link
Member Author

jbogard commented Jun 26, 2017

@lbargaoanu take a look if you don't mind

lbargaoanu
lbargaoanu requested changes Jun 26, 2017
View reviewed changes
Copy link
Member

@lbargaoanu lbargaoanu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Nitpicking. Except for the DebuggerDisplay thing.

src/AutoMapper/PathMap.cs Outdated
@@ -6,7 +6,7 @@ namespace AutoMapper
{
using Internal;

[DebuggerDisplay("{DestinationExpression}")]
[DebuggerDisplay("{" + nameof(DestinationExpression) + "}")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong :) It's supposed to show the expression and it works now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R# again...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

src/AutoMapper/Configuration/PathConfigurationExpression.cs
@@ -39,14 +44,6 @@ public void MapFromUntyped(LambdaExpression sourceExpression)

public void Configure(TypeMap typeMap)
{
//var destMember = DestinationMember;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a reminder that I left smth out, but I guess somebody will eventually complain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, should we open another GH issue then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather wait, but if you feel it matters right now...

src/AutoMapper/Configuration/PathConfigurationExpression.cs
@@ -28,6 +28,11 @@ public void MapFrom<TSourceMember>(Expression<Func<TSource, TSourceMember>> sour
MapFromUntyped(sourceExpression);
}

public void Ignore()
{
PathMapActions.Add(pm => pm.Ignored = true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is simple enough, but there is some duplication here. We could just copy Ignored to PropertyMap.Ignored and handle both cases (PathMaps and PropertyMaps) in TryPopertyMap.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might take a bit more refactoring I think, since that method returns an expression and the PropertyMap piece above in CreateAssignmentFunc does a similar filter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you would have to return null when CanResolve is false and filter for that.

@jbogard
Copy link
Member Author

jbogard commented Jun 26, 2017

@lbargaoanu reverted that R# suggestion

@jbogard jbogard merged commit c51b0e0 into master Jun 26, 2017
@jbogard jbogard deleted the ForPathIgnore branch June 26, 2017 17:57
@lock
Copy link

lock bot commented May 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lbargaoanu lbargaoanu lbargaoanu approved these changes

Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
6.1.1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jbogard @lbargaoanu