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

Fixes #10627 Inconsistencies between checkout/checkin dates on asset history and activity log #10635

Conversation

inietov
Copy link
Collaborator

@inietov inietov commented Feb 8, 2022

Description

Some time ago I add custom checkin dates to the checkin assets form, and the world were a happier place. Until users figure out that I made a mistake :(.

This PR builds over #6733 and finish the implementation of custom checkins, all was more or less ok, but I forgot to actually add the checkin_at to the logger, so every checkin found that their action_date unpopulate, making that the Activity report show the date in which it was checked in, not the custom date the user set.

Also, in the history tab of every asset details, the Bootstrap Table was using the created_at column, instead of the action_date one.

Fixes #10627

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Test Configuration:

  • PHP version: 7.4.16
  • MySQL version: 8.0.23
  • Webserver version: nginx/1.19.8
  • OS version: Debian 10

Checklist:

@JonathonReinhart
Copy link
Contributor

Until users figure out that I made a mistake :(.

Sorry!

Thanks @inietov and @snipe for rapidly addressing this! 🎉

I would argue that this is only a Bug fix and not a New feature. Thus, it qualifies to be cherry-picked into master, per this conversation with @snipe: #10629 (comment)

Once we have this in place, I can finish testing the addition of the checkin_at parameter to the /hardware/{id}/checkin API: #10552 (comment). Given how simple of a fix this is, I would love to see this in v5.3.9 also.

This would allow me to address some "bulk checkin" related things on my v5.x instances before we're ready to take the leap into v6.x (which by its nature is bound to have bugs).

@inietov
Copy link
Collaborator Author

inietov commented Feb 8, 2022

Its cool! Im grateful it got detected because i introduce the bug when added the feature.

Thats why I marked it as new feature, because in reality it was an unfinished feature? But for sure its a small change since everything was already in place.

@inietov
Copy link
Collaborator Author

inietov commented Feb 8, 2022

5.3.9 is master? If so, a PR is already submitted to master, which you can test before it gets merged if is not already. 😀

@JonathonReinhart
Copy link
Contributor

5.3.9 is master?

Yes. master is currently just 9 merge commits ahead of the v5.3.8 tag. So I'm assuming @snipe will want to take master as v5.3.9 in a couple of weeks or so.

If so, a PR is already submitted to master, which you can test before it gets merged if is not already. grinning

Okay, cool. Are you talking about #10636? Yep it looks like it is already merged. Thanks.

I will test my fix for #10552 (comment) tonight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants