[closed] Object field diff misses some data types


#1

As far as I can see, the following method needs some extension work.

AFAIK, the function is meant to receive the same field from two objects and compare them. However, if the type/ftType is any of the following, the function skips it and always return “they are not different”, regardless of the actual values:

  • Boolean
  • Image
  • List
  • Numeric (but not an integer)

And a number of more obscure field types.This is noticeable in the webtop when, after editing a piece of versioned content, the overview incorrectly displays a “The draft is identical, discard?” message as described here: Difference between DRAFT and APPROVED.

Even with this irritation, it is an awesome feature :smile:


#2

Yeah, I get confused with the boolean one all the time. I’ll have a active (or disabled) boolean for a content type. Set it. Assuming it’s a versioned content type, the overview page will say that the draft page is identical to the approved page and it is safe to delete the draft page.


#3

@mrfelna are you putting your hand up for a fix? :wink:


#4

:smile: I’m always happy to do the work. As with most of my posts, it’s
more just checking I’ve got the right idea.

Even though I’ve been using FarCry on and off for nearly three years, I
still feel like I’ve barely scratched the surface!


#5

You’re spot on, those are just plain missing :smile: It definitely needs a good once over to fill in the gaps.


#6

I’ve started to “fill in the gaps” :smile:

Just a couple of questions:

  1. Would it be better to create a PR per field type, or add all types then submit one big PR?
  2. I plan on adding at least image, numeric and list to the comparison types. Any requests for other field types?
  3. Would it be good to try and add a generic catch all?

#7

What ever works for you :slight_smile:

PRs that can be merged quickly are always welcome. Release each isolated improvement if its likely you’ll need a lot of time to get the whole thing done.

Maybe there’s option for serialising stobj and comparing the entire object minus datetimelastupdated, lockedby and any other system properties that might generate false positives.


#8

All formtools need to be covered, but there will be many formtools that are just strings (image, file, state, country, etc) and could probably be handled by a single case inside getPropertyDiff(), e.g. they could be added to the existing “string,longchar” case:

Some could be ignored such as creditcard* since it’s unlikely those are ever stored, they are usually used in components which extend farcry.core.packages.forms.forms and so are transient.


#9

Many thanks @mrfelna


#10

No problem :smile:

Still got some types to add, but no idea when / if I’ll have the time :wink:


#11

Oops - thanks for the fix :smile: