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

[15.0][MIG] web_archive_date #686

Merged
merged 7 commits into from
Aug 29, 2023

Conversation

JasminSForgeFlow
Copy link
Contributor

Standard Migration

@ForgeFlow

Depends on:

@JasminSForgeFlow JasminSForgeFlow force-pushed the 15.0-mig-web_archive_date branch from 72485dc to 75ebc4e Compare July 4, 2023 08:58
@AaronHForgeFlow
Copy link
Contributor

Hi @JasminSForgeFlow , the base module is merged. Can you rebase?

@JasminSForgeFlow
Copy link
Contributor Author

Hi @JasminSForgeFlow , the base module is merged. Can you rebase?

Its done already, Thanks

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

I expected to see the archive date and user in the view metadata info of an archive record, but it is missing:

2023-07-05_08-01

@JasminSForgeFlow JasminSForgeFlow force-pushed the 15.0-mig-web_archive_date branch from 75ebc4e to 3aeddc3 Compare July 5, 2023 10:44
@JasminSForgeFlow
Copy link
Contributor Author

I expected to see the archive date and user in the view metadata info of an archive record, but it is missing:

2023-07-05_08-01

Its Fixed, Thanks

Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Could you cherry-pick the fix done here #687?

Also, your commit history only contains the pre-commit commit, you need to have the pre-commit and migrations changes separated in 2 different commits to ease review. Could you do that?

GuillemCForgeFlow and others added 4 commits July 6, 2023 08:59
Glue module between base_archive_date and web to display the Latest Archived Date and Latest Archived by on the metadata of a record.
@JasminSForgeFlow JasminSForgeFlow force-pushed the 15.0-mig-web_archive_date branch from 3aeddc3 to dec93fb Compare July 6, 2023 03:40
@JasminSForgeFlow
Copy link
Contributor Author

Could you cherry-pick the fix done here #687?

Also, your commit history only contains the pre-commit commit, you need to have the pre-commit and migrations changes separated in 2 different commits to ease review. Could you do that?

#687 included and commits also fixed.
Thanks

@JasminSForgeFlow JasminSForgeFlow force-pushed the 15.0-mig-web_archive_date branch from dec93fb to 71e6756 Compare July 6, 2023 03:43
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Functional review LGTM 👍

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

Functional review

Copy link
Contributor

@GuillemCForgeFlow GuillemCForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏽

Comment on lines +1 to +3
Format of `archive_date` is not the same as `write_date` shown in the metadata. Values are formatted in javascript. Can't inherit the method and adapt the value there.

TO BE DONE: Check how to format the value in the QWeb view somehow.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Format of `archive_date` is not the same as `write_date` shown in the metadata. Values are formatted in javascript. Can't inherit the method and adapt the value there.
TO BE DONE: Check how to format the value in the QWeb view somehow.

You can remove the file. Thank you @JasminSForgeFlow for the Javascript changes! 👍🏽

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, Thanks

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@JasminSForgeFlow JasminSForgeFlow force-pushed the 15.0-mig-web_archive_date branch from 71e6756 to 74e0464 Compare July 10, 2023 06:23
@hbrunn
Copy link
Member

hbrunn commented Aug 29, 2023

/ocabot migration web_archive_date
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Aug 29, 2023
@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-686-by-hbrunn-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 90afff5 into OCA:15.0 Aug 29, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at fafbe55. Thanks a lot for contributing to OCA. ❤️

@GuillemCForgeFlow GuillemCForgeFlow deleted the 15.0-mig-web_archive_date branch August 29, 2023 09:09
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.

6 participants