Page MenuHomePhabricator

Allow to add and edit Phrequent entries manually
Open, WishlistPublic

Assigned To
None
Authored By
witrin
May 21 2014, 8:02 PM
Referenced Files
F619326: prequent-edit.tar.gz
Jul 13 2015, 5:46 PM
F604969: prequent-edit.tar.gz
Jul 10 2015, 8:39 PM
F304798: create.png
Feb 14 2015, 7:23 PM
F304797: edit.png
Feb 14 2015, 7:23 PM
Tokens
"Like" token, awarded by arcadien."Like" token, awarded by michelkaeser."Like" token, awarded by mirrom."Like" token, awarded by Aronnax."Like" token, awarded by tiguchi."Like" token, awarded by CodeMouse92.

Description

Sometimes we forget the running time recording and thus false values were ​​stored. Therefore, it is necessary to correct the own times manually. Also, there are users who prefer to manually enter their time without using the start/stop function.

Revisions and Commits

Event Timeline

witrin raised the priority of this task from to Needs Triage.
witrin updated the task description. (Show Details)
witrin added a project: Phrequent.
witrin added a subscriber: witrin.

Sorry we have updated yesterday and just noticed that the start/stop function has changed. So a manual add function seems to be no longer necessary. But I would still say that an edit/delete function for the own tracking might be helpful.

witrin triaged this task as Normal priority.May 22 2014, 11:11 PM
chad lowered the priority of this task from Normal to Wishlist.
chad removed a subscriber: epriestley.

I would say that this feature alone (the lack of editing) is what completely prevents my company's use of Phrequent. I would even go as far as to say that, in the way of time-tracking in a business environment, it makes Phrequent completely non-viable.

I propose (though I would not do so myself, lest I overstep my bounds) this be raised to a higher priority.

Well I'm a little bit confused about the last actions from chad. Because when you take a look at https://secure.phabricator.com/D10162#86393 you'll see that epriestley was willing to pull another feature in this context. So I tried to do that in https://secure.phabricator.com/D10674 but I got no reaction since that and thus unfortunately I don't see any effort in spending more time in this feature currently.

The sad part here is, I already implemented this feature. It might have only one bug: You might be able to create or edit time entries for other users; I'm currently not really sure about it :). By eliminating this little bug the feature seems totally fine to me and brings a real effort because currently you can't create time entries which are already done (which happens sometimes in our company). And since you should only able to create/edit your own entries I don't see any need for a transaction history of these actions.

Hi @witrin, while I can't account for any of the above, I will say one thing in response to your last statement - transaction history is VERY important to managers, especially where keeping weekly time logs is concerned.

Our current time entry system has the #1 flaw of allowing anyone to edit anyone's time, so that bug would prevent me patching. If it were fixed, however, it might be worth it to patch it in.

Hey @CodeMouse92, I've checked this and in fact you're not able to create/edit entries for other user through the UI; the first field is pre-filled (on create) and read-only (on edit and create):

edit.png (647×1 px, 44 KB)

create.png (647×1 px, 44 KB)

The first field is always the current user, that's all. Indeed I haven't checked if somebody able to hack this. The responsible code is this, so maybe you're able hack the edit action but maybe @epriestley could say something about it:

if ($is_create) {
  $usertime = id(new PhrequentUserTime())
    ->setUserPHID($user->getPHID());
} else {
   $usertime = id(new PhrequentUserTimeQuery())
     ->setViewer($user)
     ->requireCapabilities(
       array(
         PhabricatorPolicyCapability::CAN_VIEW,
         PhabricatorPolicyCapability::CAN_EDIT,
       ))
     ->withIDs(array($this->id))
     ->executeOne();
   if (!$usertime) {
     return new Aphront404Response();
   }
}

So, a user is only be able to create/edit entries for himself through the UI and thus I don't see any effort for a transaction history of the create and edit actions (but maybe there is a misunderstanding).

Hi @witrin, I just had an issue this week with someone who really needed to edit a time entry that got mangled. Would you mind reviewing your add/edit code given the latest codebase in Phrequent, and creating a new diff? Perhaps it'll get accepted, plus I can at least patch it in to my own copy. (I hope the rest of ya'll don't mind my saying that, but I'm pretty darned desperate for this.)

@CodeMouse92 The code is running in a production environment on the latest codebase. The problem is just this feature is bundled together with two other features in one branch and I really don't know how to select a lose set of commits using arc diff. I've tried once but I was only able to use the latest nth and that's not helping here. So this would mean I've to create a patch clone the repository download the older patch from the revision put them together and then I could use arc diff (but without the tests because I've no development environment). So when you could help here a little bit, I could send you the patches and you could do the rest (publishing them)?

Wait this is even worse. I think I haven't published the add/edit feature ever separately. But this doesn't change my offer: sending you a patch and you could do the rest...

I'm still rusty on my PHP, but we might be able to work together and pull it off. I think it would be fantastic to get that on Phabricator so this task can be marked as done! We might just need to message each other on Conpherence if I get stuck on implementing your patch.

I just tried to apply https://secure.phabricator.com/D10162 to current stable but it doesn't work really. It seems a little has changed in Phabricator. Edit function throws an exception and PhrequentSearchEngine::renderResultList() returns a PhabricatorApplicationSearchResultView object in current code but nor in the patch. Unfortunately I'm very new to Phabricator so I'm not able to fix that easily.

@witrin do you mind to send me the patch you mentioned? I would like to test it and I would like to see Phrequent to be more usable. Maybe I can help too.

@Aronnax @CodeMouse92 I'll try to upload the patch asap when I find some time. Currently I'm pretty bussy.

@Aronnax @CodeMouse92 Here you go. Hope that's all ...

Hi @witrin. Thanks for the files!

Creating an time entry works. What is missing is editing. Looking at your original patch I see that stuff from query/ and storage/ is missing. there was also a part I couldn't get working because of api changes.

Do you mind to pack the whole phrequent folder?

My goal would be to make a create/edit patch. And later make a patch for your excel export too.

@Aronnax, you need to download a fresh copy of the Git repository yourself, and then look under applications/phrequent.

@witrin, thank you very much!

@CodeMouse92 I may have not been clear what I meant. In prequent-edit.tar.gz are some files missing to make editing work. So @witrin may pack his phrequent folder so we can get the missing pieces and make it patch out of it.

@Aronnax You're right I've forgot the adaptions in the listing:

.

avivey renamed this task from Allow to add and edit entries manually to Allow to add and edit Phrequent entries manually.Jul 13 2015, 6:23 PM

Now I have the edit function. Thanks.
There's a slight misbehaviour that the users time format is not respected. I'll try to change that first...

It seems that AphrontFormDateControl is responsible for the wrong time formatting and not the phrequent code.

any news here? the implementation seemed on a good way but there was no update since July 2015

I just applied the patch (roughly) and the "edit" pen appeared in Phrequent. By the way, clicking on it lead to a 404 Not Found error page.

Thanks for link, i know that page already. This feature is the one missing for us to switch from another well-known system which have more and more performance and billing problems.
That means we can finish the development of this one : that's why i asked for a status, rather than a boring "will it be ready next week" :)