search

appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
33.95k stars 3.66k forks source link

[Bug]: Reverted temporary changes made to a codebase that were never pushed were shown in the change diff in Bitbucket #36486

Closed MaxSunlife closed 6 days ago

MaxSunlife commented 1 week ago

Is there an existing issue for this?

Description

I hard coded a value to test something, but reversed the change before I pushed to the repo because that test value was never meant to be part of the update (only a test). However, when I pushed to the repo the change was visible in the repo's change diff page even though it was never pushed. After pushing we pulled to a different environment and confirmed that change was not part of the codebase. So why is it still in the change diff if that change was never pushed and isn't part of the repo?

image

Steps To Reproduce

  1. make an intended change
  2. make a separate temporary change
  3. revert the temporary change
  4. push intended change to the repo
  5. check the change dif to find the temporary change that was never pushed

Public Sample App

No response

Environment

Production

Severity

Critical (Broken Production apps)

Issue video log

No response

Version

Self-Hosted v1.19

Nikhil-Nandagopal commented 1 week ago

@MaxSunlife did you commit the temporary change? If so, all committed changes are automatically pushed and that could explain how this happened.

MaxSunlife commented 1 week ago

Hi Nikhil,

No, that’s the weird thing about it. There were 2 pushes that were part of this change’s branch.

  1. Was an initial push after making no changes. Immediately after the new branch was created and discard / pull was hit there were 2 changes that needed to be pushed. Likely these were Appsmith specific, we’ve encountered this before, where Appsmith requires updates to projects as part of the tool itself. After I pushed those the new branch showed itself to be up to date. This push was made before any changes happened to the branch. Because of how opaque the Appsmith git implementation is though we cannot even see which parts of the project these (or any) changes took place in. It only tells us the number of items and their general type (like query or page).
  2. The second push was to 2 JavaScript files (commented out some problem code). There were changes the tool pushed to the page jsons as a result of this but nothing manual was changed there.

Between 1 and 2 I also changed 2 queries to have a hard coded value to test with. I undid those changes before I pushed the JS file updates. Because there was a net zero change between pushes though this should have been invisible to bitbucket. 1 of those 2 hard coded value changes made for testing purposes still shows up in the change diff after the 2nd push though, even though that change is not in the code base itself and is not present when we deploy the code to other environments. This is the oddity we want to investigate.

Do you have time to hop on a call this afternoon to take a look with us in the bitbucket repo?

From: Nikhil Nandagopal @.> Sent: Monday, September 23, 2024 2:23 PM To: appsmithorg/appsmith @.> Cc: Maxwell Wallace @.>; Mention @.> Subject: Re: [appsmithorg/appsmith] [Bug]: Reverted temporary changes made to a codebase that were never pushed were shown in the change diff in Bitbucket (Issue #36486)

CAUTION This email originated from outside the organization. Please proceed only if you trust the sender.

@MaxSunlifehttps://github.com/MaxSunlife did you commit the temporary change? If so, all committed changes are automatically pushed and that could explain how this happened.

— Reply to this email directly, view it on GitHubhttps://github.com/appsmithorg/appsmith/issues/36486#issuecomment-2369042846, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BLQTLVBBYBQJ6Y43464WVWDZYBL75AVCNFSM6AAAAABOWKEEMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRZGA2DEOBUGY. You are receiving this because you were mentioned.Message ID: @.**@.>>

This e-mail message (including attachments, if any) is intended for the use of the individual or entity to which it is addressed and may contain information that is privileged, proprietary, confidential and exempt from disclosure. If you are not the intended recipient, you are notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify the sender and erase this e-mail message immediately.

Le présent message électronique (y compris les pièces qui y sont annexées, le cas échéant) s'adresse au destinataire indiqué et peut contenir des renseignements de caractère privé ou confidentiel. Si vous n'êtes pas le destinataire de ce document, nous vous signalons qu'il est strictement interdit de le diffuser, de le distribuer ou de le reproduire. Si ce message vous a été transmis par erreur, veuillez en informer l'expéditeur et le supprimer immédiatement.

Nikhil-Nandagopal commented 1 week ago

@MaxSunlife ok I understand what you are experiencing now. The changes you are seeing are in the jsonPathKey variable which is actually calculated on the fly and does not need to be tracked via git at all. We can can probably pick up not tracking it as an enhancement but essentially what happened here is that the variable did not get recomputed but did get tracked and therefore showed an older value. I assume you are not seeing any discrepency in the behavior of the application.

MaxSunlife commented 6 days ago

Thank you Nikhil, We decided to roll back the change in BitBucket and redo it on a new clean branch. However, it's encouraging to hear that you will fix this in an enhancement. Thanks!