audited icon indicating copy to clipboard operation
audited copied to clipboard

reduce repeated/unnecessary code

Open grosser opened this issue 6 years ago • 6 comments

just wanted to make a tiny change ... then things escalated 😭 @danielmorrison @domcleal

  • make audited_attributes and audited_changes follow the same logic regarding only & except and privacy level
  • remove runtime .to_s-ing of only & except
  • remove duplicate non_audited_columns instance method
  • remove non_audited_columns class method in favor of either calling audited except: [] or modifying audited_options
  • remove unnecessary .flatten
  • test actual auditing behavior instead of testing class accessors
  • test actual recorded columns instead of testing for if they include certain things to make tests catch empty hashes / wrong keys

warning: records_changes_to_specified_fields? seems to be untested ... but that's for another PR

grosser avatar Jun 16 '17 00:06 grosser

... I'd love to take this even further and make create + destroy also simply audit changes ... that would unify the stored data and remove redundant fields in create/destroy audits ...

grosser avatar Jun 16 '17 01:06 grosser

Looks good to me after a quick glance. Should we add any deprecation methods/warnings?

danielmorrison avatar Jun 16 '17 14:06 danielmorrison

warnings just get ignores ... if someone updates this will blow up and they have to find out what to do ... could keep the method defined and raise a NotImplementedError instead though ?

grosser avatar Jun 16 '17 15:06 grosser

added NotImplementedError for removed methods ... good to go ?

grosser avatar Jun 18 '17 15:06 grosser

@danielmorrison

grosser avatar Jun 19 '17 20:06 grosser

@grosser I think a lot of the complexity and duplication has been resolved in various other PRs that were recently merged. Are there some changes that are still required? If so, could you please update your PR?

tbrisker avatar Mar 14 '18 14:03 tbrisker