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

[IMP] project_timesheet_time_control: Tell the user no running line was found #596

Merged

Conversation

yajo
Copy link
Member

@yajo yajo commented Oct 15, 2019

Imagine this scenario:

  1. In tab 1 of the browser, you have opened task 1.
  2. In tab 2 of the browser, you have opened task 2.
  3. You go to tab 1 and start a timer.
  4. Work, work, work...
  5. You go to tab 2 and start a timer, stopping that of task 1.
  6. Work, work, work...
  7. You go to tab 1 and see that timer as running (it is not, but you didn't refresh). You hit stop.

Before this commit, it just seemed like the timer was actually stopped. What did happen behind the scenes is that your view was refreshed, but no timer was touched fortunately.

After this commit, you get a message telling you that there's no timer to stop and that your browser is most likely out of sync. This mimics the behavior previously found when doing the same, but directly in the AAL. Now it's present in projects and tasks too.

@Tecnativa TT19205 OCA/timesheet#280

@yajo yajo changed the title [IMP] Tell the user no running line was found [IMP] project_timesheet_time_control: Tell the user no running line was found Oct 15, 2019
…as found

Imagine this scenario:

1. In tab 1 of the browser, you have opened task 1.
2. In tab 2 of the browser, you have opened task 2.
3. You go to tab 1 and start a timer.
4. Work, work, work...
5. You go to tab 2 and start a timer, stopping that of task 1.
6. Work, work, work...
7. You go to tab 1 and see that timer as running (it is not, but you didn't refresh). You hit stop.

Before this commit, it just seemed like the timer was actually stopped. What did happen behind the scenes is that your view was refreshed, but no timer was touched fortunately.

After this commit, you get a message telling you that there's no timer to stop and that your browser is most likely out of sync. This mimics the behavior previously found when doing the same, but directly in the AAL. Now it's present in projects and tasks too.
@yajo yajo force-pushed the 12.0-project_timesheet_time_control-ux_love branch from df5ed82 to d9e5e37 Compare October 15, 2019 08:10
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

This is what I told you it was happening!

Please include POT + es.po updates as well, and I'll fast-track

@pedrobaeza pedrobaeza added this to the 12.0 milestone Oct 15, 2019
@pedrobaeza
Copy link
Member

Also there's no possibility of avoiding code duplication?

@yajo
Copy link
Member Author

yajo commented Oct 15, 2019

Maybe with a mixin. Do you think it's worth it?

@pedrobaeza
Copy link
Member

Well, I'm seeing a lot of code duplication, also in OCA/timesheet#280, so I think we can explore that possibility

@yajo
Copy link
Member Author

yajo commented Oct 15, 2019

Can we merge and leave that for later? 🙄

@pedrobaeza
Copy link
Member

I prefer to do it now.

Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.
yajo added a commit to Tecnativa/timesheet that referenced this pull request Oct 15, 2019
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
@yajo
Copy link
Member Author

yajo commented Oct 15, 2019

OK, done.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Thank you very much. This is much lighter! Fast-tracking it as said.

@pedrobaeza
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-596-by-pedrobaeza-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 15, 2019
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit c02e49e into OCA:12.0 Oct 15, 2019
@OCA-git-bot
Copy link
Contributor

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

@yajo yajo deleted the 12.0-project_timesheet_time_control-ux_love branch October 16, 2019 10:37
manuelcalerosolis pushed a commit to Tecnativa/timesheet that referenced this pull request Feb 27, 2020
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
pedrobaeza pushed a commit to Tecnativa/timesheet that referenced this pull request Sep 26, 2020
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
fshah-initos pushed a commit to initOS/timesheet that referenced this pull request Mar 15, 2021
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
fshah-initos pushed a commit to initOS/timesheet that referenced this pull request Mar 31, 2021
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
hkapatel-initos pushed a commit to initOS/timesheet that referenced this pull request Sep 3, 2021
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
LudLaf pushed a commit to Tecnativa/timesheet that referenced this pull request Nov 21, 2022
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
pedrobaeza pushed a commit to Tecnativa/timesheet that referenced this pull request Nov 27, 2022
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
carolinafernandez-tecnativa pushed a commit to Tecnativa/timesheet that referenced this pull request Sep 25, 2023
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
carolinafernandez-tecnativa pushed a commit to Tecnativa/timesheet that referenced this pull request Sep 25, 2023
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
mde-scopea pushed a commit to mde-scopea/timesheet that referenced this pull request Jan 31, 2024
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
RogerSans pushed a commit to sygel-technology/timesheet that referenced this pull request Jul 29, 2024
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
jdebetaz pushed a commit to jdebetaz/oca-timesheet that referenced this pull request Jan 15, 2025
Models related to timesheet time controls now inherit from a mixin that adds most needed logic automatically.

This requires the changes introduced in OCA/project#596.
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