bug-fix: duplicating copy right year

[Gabe-Torres]

add: "data: { turbo: "false" }" on adoptable pet link.

🔗 Issue

(#1367)

✍️ Description

After looking into this bug, I found that Turbo caches previous pages for faster navigation/refreshes, which was causing the duplication problem I think. Adding data: { turbo: "false" } to the adoptable pet link in the organization home index view fixed this by forcing a full page refresh when navigating back.

If this is the approach we want to take, there may be other instances where this is needed. I’m also curious about the extent to which the application relies on Turbo. Could disabling Turbo in this way cause any unintended issues?

📷 Screenshots/Demos

https://jam.dev/c/09dca437-db81-46da-ab55-1fce93fb9bdd

[jmilljr24]

@Gabe-Torres I think the issue is going to come up anywhere the footer partial is used (which link you click and go back from shouldn't matter). We should leave turbo enabled.

Can you take a look at the two partials where the copyright is. layouts/shared/_footer and _no_tenant_footer. The append is probably the problem.

<span id="copyright"> <script>document.getElementById('copyright').appendChild(document.createTextNode(new Date().getFullYear()))</script> </span>

Maybe something like this will do the trick:
document.getElementById('copyright').innerHTML = new Date().getFullYear()

[MooseCowBear]

@jmilljr24 Maybe there's something I'm missing but is there a reason to prefer a script here? Couldn't we use Time.now.year to display the current year?

[jmilljr24]

@jmilljr24 Maybe there's something I'm missing but is there a reason to prefer a script here? Couldn't we use Time.now.year to display the current year?

That's definitely better! The script was already being used and I didn't look into it further. I can't think of a good reason to use the inline script. Thanks for pointing it out.

[Gabe-Torres]

@jmilljr24 Maybe there's something I'm missing but is there a reason to prefer a script here? Couldn't we use Time.now.year to display the current year?

Thank you @MooseCowBear. That does indeed fix the issue! 👍🏽

[jmilljr24]

Just some leftover changes from your previous troubleshooting that we don't need.

[jmilljr24]

Suggested change

	<%= link_to adoptable_pet_path(pet), data: { turbo: "false" } do %>
	<%= link_to adoptable_pet_path(pet) do %>

[Gabe-Torres]

I thought I took it out! Sorry about that. I’ve just removed it, and I believe everything is updated now. My commit history is a bit messy on this branch. For future reference, what is the best way to clean it up? I’d like to make sure that only the relevant commits are included in the push, with any unnecessary ones removed.

[jmilljr24]

@Gabe-Torres for this repo we do a squash and merge into main so it really doesn't matter. You can take a look at the commit history for main and see what is actually in there. Other organizations may have a different policies on what commits that want to in the history.

From your end, look up interactive rebase if you really want to clean up your commits before opening a PR. After the PR is open I generally wouldn't changing your commit history.

[Gabe-Torres]

I'm going to close this branch and make a new one. I got into a bit of a Git mess and think it'll be better to start over for such a small change. 😨

EDIT - new pr that looks much cleaner - #1388