Skip to content

FOUR-19110: [40531] changes on screen are not reflected in ongoing cases#7427

Merged
ryancooley merged 3 commits intonextfrom
bugfix/FOUR-19110
Sep 29, 2024
Merged

FOUR-19110: [40531] changes on screen are not reflected in ongoing cases#7427
ryancooley merged 3 commits intonextfrom
bugfix/FOUR-19110

Conversation

@eiresendez
Copy link
Copy Markdown
Contributor

@eiresendez eiresendez commented Sep 26, 2024 •
edited
Loading

Issue & Reproduction Steps

  • Have cases running in a process.
  • edit one of the screens that is used in those cases' process.
  • save and publish the changes on the screen.
  • Changes are reflected only in new cases.

Current behavior

  • Changes applied on a screen and published are only reflected in new cases. in 4.10 the changes were reflected in new and also ongoing cases

Expected behavior

  • Change should be reflected in ongoing cases.

Solution

  • Added a version parameter (screen_version) to screen URLs, generated based on the latest published id of the screen.
  • This approach forces the browser to treat each updated published screen as a new resource, preventing the use of outdated cached versions.

How to Test

  • Verify that screen updates generate new URLs with different version parameters, ensuring that changes are correctly fetched.
  • Test across different browsers to confirm that old cached versions are not used.

Related Tickets & Packages

Code Review Checklist

  • I have pulled this code locally and tested it on my instance, along with any associated packages.
  • This code adheres to ProcessMaker Coding Guidelines.
  • This code includes a unit test or an E2E test that tests its functionality, or is covered by an existing test.
  • This solution fixes the bug reported in the original ticket.
  • This solution does not alter the expected output of a component in a way that would break existing Processes.
  • This solution does not implement any breaking changes that would invalidate documentation or cause existing Processes to fail.
  • This solution has been tested with enterprise packages that rely on its functionality and does not introduce bugs in those packages.
  • This code does not duplicate functionality that already exists in the framework or in ProcessMaker.
  • This ticket conforms to the PRD associated with this part of ProcessMaker.

ci:next
ci:deploy
ci:screen-builder:bugfix/FOUR-19110

@tokensPM
Copy link
Copy Markdown

tokensPM commented Sep 27, 2024

QA server K8S was successfully deployed https://ci-366b9da8b8.engk8s.processmaker.net

@caleeli
Copy link
Copy Markdown
Contributor

caleeli commented Sep 27, 2024

Seems there is no cache in the screen:

ScreenCache.mov

@tokensPM
Copy link
Copy Markdown

tokensPM commented Sep 27, 2024

QA server K8S was successfully deployed https://ci-366b9da8b8.engk8s.processmaker.net

@ryancooley
Copy link
Copy Markdown
Contributor

ryancooley commented Sep 27, 2024

@caleeli Is this not the intended behavior? It seems as if it handled the situation correctly. First it returns a 200, then it returns a 304 not modified. cc: @eiresendez

danloa
danloa approved these changes Sep 27, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@danloa danloa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had some issue, probably web browser related. It used the disk cache to retrieve the screen. After I disabled and enabled the browser cache, everything is working correctly now.

@eiresendez
Revert "fix: implement Last-Modified header with If-Modified-Since va…
a168f7e 
…lidation for cache control"

This reverts commit 1858009.
@eiresendez
feat: implement cache-busting with version parameter to ensure screen…
d8186ea 
… updates are reflected

- Added a version parameter to screen URLs based on the last modified id.
- Ensures that changes in the screen trigger a new URL, bypassing cached versions.
- Prevents stale data by forcing the browser to fetch the updated resource when changes occur.
@eiresendez eiresendez mentioned this pull request Sep 27, 2024
Merged
@tokensPM
Copy link
Copy Markdown

tokensPM commented Sep 27, 2024

QA server K8S was successfully deployed https://ci-366b9da8b8.engk8s.processmaker.net

devmiguelangel
devmiguelangel approved these changes Sep 27, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@devmiguelangel devmiguelangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-09-27 at 3 38 17 PM Screenshot 2024-09-27 at 3 38 47 PM

@ryancooley ryancooley merged commit d51708c into next Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devmiguelangel devmiguelangel devmiguelangel approved these changes

@danloa danloa danloa approved these changes

@nolanpro nolanpro Awaiting requested review from nolanpro

@caleeli caleeli Awaiting requested review from caleeli

@ryancooley ryancooley Awaiting requested review from ryancooley

Assignees

No one assigned

Labels

4.11.4

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@eiresendez @tokensPM @caleeli @ryancooley @devmiguelangel @danloa @boliviacoca