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.

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

Missing change log Difference after upgrade to 2.11 for pre 2.11 changes #6493

Closed
netsandbox opened this issue May 27, 2021 · 13 comments · Fixed by #6552
Closed

Missing change log Difference after upgrade to 2.11 for pre 2.11 changes #6493

netsandbox opened this issue May 27, 2021 · 13 comments · Fixed by #6552
Assignees
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@netsandbox
Copy link
Contributor

NetBox version

v2.11.4

Python version

3.8

Steps to Reproduce

  1. create a object in NetBox 2.10.x
  2. change the object to create a "Updated" change log record
  3. upgrade NetBox to 2.11.4
  4. view the change log record created in the previous NetBox version

Expected Behavior

Change log record should show the changes (Differences).

Observed Behavior

Change log record doesn't show the changes (Differences).

NetBox 2.10.10:
netbox_2 10 10_changelog

NetBox 2.11.4:
netbox_2 11 4_changelog

@netsandbox netsandbox added the type: bug A confirmed report of unexpected behavior in the application label May 27, 2021
@ziggekatten
Copy link

We observe the same issue when testing upgrade from 2.10.10 to 2.11.3

This will definitively stop us in or tracks at the moment.

@jeremystretch
Copy link
Member

This is working as intended. The changelog was overhauled in NetBox v2.11 to capture atomic changes as discussed in the release notes.

You haven't lost any data by upgrading: The old change records are still there and can be inspected. The only thing missing is the delta automatically comparing a v2.10-format change record to its predecessor for convenience. Of course, this was often misleading anyway (hence the improvement in v2.11).

@netsandbox
Copy link
Contributor Author

netsandbox commented May 27, 2021

@jeremystretch

You haven't lost any data by upgrading: The old change records are still there and can be inspected.

Where in the WebUI can I see the old changelog data?

@jeremystretch
Copy link
Member

The same place you're looking now. The changelog implementation prior to v2.11 did not capture pre-change data, only post-change data. You have to compare two pre-v2.11 change records to see the differences (which is all the UI was doing under v2.10).

@netsandbox
Copy link
Contributor Author

Ok, understand.
For 2.11 changes I see it on one page and for pre 2.11 changes I have to compare the last change with the previous one.
This isn't very user friendly 😢
Actually the change log is very important for us (this is why we set CHANGELOG_RETENTION = 0) to inspect who has changed what on our hosts.

Any chances this can be improved?

@drmsoffall
Copy link

As a fellow "retention = 0" believer, I agree this change is a bit jarring. I don't disagree that the old delta views were at times misleading or confusing, but having some thousands of changes that no longer have a delta view is confusing (to end users) as well.

My suspicion is that it would be a couple of comparatively-simple updates to the view and template to get this behavior back for legacy changes, but I'm not sure if that's what the BDFL wants based on his earlier comments. 😄

@jeremystretch jeremystretch added the status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation label Jun 2, 2021
@jeremystretch
Copy link
Member

I've marked this as needs owner for anyone who would like to take a stab at it. Please just bear in mind that any potential solution would need to differentiate to the user when they are seeing an atomic change (post-v2.11) versus the comparison of two different records.

@netsandbox
Copy link
Contributor Author

I think a upgrade script which sets Pre-Change Data from the previous change record Post-Change Data would be better.
This way we don't need any additional code for the old change records format, and all change records have the same format.

@jeremystretch
Copy link
Member

@netsandbox we can't do that, because it's not strictly the same data as in post-v2.11, and would be a misrepresentation of the context in which the change is being viewed.

@jeremystretch
Copy link
Member

I should expand on my comment above. Suppose, under v2.10, the following actions occur:

  1. User makes a change to an object, which gets logged
  2. That same object is changed via some external mechanism (e.g. someone working inside nbshell)
  3. User makes a second change to the object

Under v2.10 and earlier, comparing the last change to the earlier change will show the change from step 2 as well. (This deficiency is why we overhauled change logging in v2.11.)

If the same set of actions were to occur in v2.11, the changes from steps 1 and 3 will be shown atomically, each attributed to the appropriate user. We can't "fake" this for older changes, because it would attribute the change from step 2 to the user responsible for step 3 without acknowledging the caveat.

@drmsoffall
Copy link

A migration or script seems obvious, but it would clutter the database and make it difficult to know whether a record is atomic without adding another field. It makes sense to implement this in the UI.

I would propose the following logic: if the ObjectChange contains post-change data but no pre-change data and a previous change exists, generate a diff between the post-change data and the previous change's post-change data. This effectively replicates the v2.10 logic for cases where it could be useful.

On differentiating:

  • At a minimum, the Pre-Change Data panel should remain unpopulated to convey that the diff is not atomic; the text "None" could be replaced with "None - Comparing to previous change" to add clarity
  • Additional indications could be included like a "Change record type" row in the Change fields panel or a conditional modification to the title of the Difference panel, depending on how much emphasis is desired

Thoughts?

@jeremystretch
Copy link
Member

if the ObjectChange contains post-change data but no pre-change data and a previous change exists, generate a diff between the post-change data and the previous change's post-change data.

Yes, with an additional condition: do this only when dealing with an update or delete event; not create. (Though of course a create event should never have a preceding change record in the first place.)

I think it's sufficient to just display a mild warning message when comparing to a previous change. Something along the lines of:

Warning: Comparing non-atomic change to previous change record ($ID/$date).

@drmsoffall
Copy link

Sounds reasonable to me. I can give this a shot if you'd like.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jun 4, 2021
drmsoffall pushed a commit to drmsoffall/netbox that referenced this issue Jun 5, 2021
drmsoffall pushed a commit to drmsoffall/netbox that referenced this issue Jun 5, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants