#7852 Don't update mod time when viewing artifact creates a cache

v1.3.0
closed
sf-2 (994)
General
2015-08-20
2015-03-12
No

In addition to the fixes done in [#7834], a ticket can still be marked as updated just when viewed. This happens if the cache was not created initially (e.g. ticket import - no view, or first view was under the threshold time) and then the cache was created upon a later view. We need to skip updating mod_date and last_updated when a cache is set. I think it's probably safe to set the skip_mod_date and skip_last_updated flags on the session, since there shouldn't be cases of rendering a markdown (viewing) in the same session as legitimately updating records (I hope).

Related

Tickets: #7834

Discussion

  • Dave Brondsema

    Dave Brondsema - 2015-04-06
    • labels: --> sf-current, sf-2
     
  • Heith Seewald

    Heith Seewald - 2015-04-07
    • assigned_to: Heith Seewald
     
  • Heith Seewald

    Heith Seewald - 2015-04-07
    • status: open --> in-progress
     
  • Heith Seewald

    Heith Seewald - 2015-04-14

    Ready for review.

    hs/7852

     
  • Heith Seewald

    Heith Seewald - 2015-04-14
    • status: in-progress --> review
     
  • Dave Brondsema

    Dave Brondsema - 2015-04-15
    • status: review --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2015-04-15

    Very minor feedback:

    • In both places, setattr(session, 'skip_mod_date', True) can be session.skip_mod_date = True
    • In test_cached_convert, we should have a comment explaining why skip_mod_date is needed to avoid the automatic setting of mod_date when it is saved

    Bigger:

    • Since cached_convert is being mocked, there's the caching logic will never run at all. We need it to run so the test actually tests it :) I'd just remove that mock. For a strong assertion that it did run, rather than mocking it and checking call_count, we could check that ticket.description_cache.html is set... but that requires querying Ming for the record and making sure you get that latest (not cached in the Ming layer)

    Besides that I think the test approach is fine, even if it's not ideal.

     
  • Heith Seewald

    Heith Seewald - 2015-04-16
    • status: in-progress --> review
     
  • Heith Seewald

    Heith Seewald - 2015-04-16

    Force-pushed hs/7852 (rebase)

    Good feedback.

    I removed the mock am now asserting ticket.description_cache.html returns the expected result.

     
  • Dave Brondsema

    Dave Brondsema - 2015-04-16
    • status: review --> closed
    • Reviewer: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2015-04-20
    • labels: sf-current, sf-2 --> sf-2
     
  • Igor Bondarenko

    Igor Bondarenko - 2015-06-18
    • Milestone: unreleased --> asf_release_1.3.0
     

Log in to post a comment.