Migrated commit_represent to S3Represent #1182
Conversation
modules/s3db/req.py
Outdated
| """ | ||
| table = current.s3db.req_commit | ||
| if row.type == 1: | ||
| return "%s - %s" % (table.site_id.represent(row.site_id), |
There was a problem hiding this comment.
Makes no difference with S3Represent if you call other represents per-row.
There was a problem hiding this comment.
You should call the bulk-method of the same (!) represent instances once during lookup_rows for /all/ values.
That will store the representations in the instance, so that when you call them per-row in represent_row, they wouldn't be looked up again.
| """ | ||
|
|
||
| table = current.s3db.req_commit | ||
| super(req_CommitRepresent, self).__init__() |
There was a problem hiding this comment.
This constructor does basically nothing - so should be removed.
|
This is the right concept - but a sloppy implementation of it ;) You should always test your code before submitting a PR - and this code can not work, so you either didn't test it, or you ignored that it doesn't work ;) |
|
Actually @nursix I did tried to test it, but I think they way I tested it is wrong, sorry for that. |
|
@anurag-ks - generally no problem to ask for feedback on the concept by tagging me in a comment to your commit before submitting a PR. But for a PR, stuff should be working. You could try to write a unit test for this (even if just as an exercise) - this would give you immediate feedback while refactoring things. |
|
@nursix just a doubt, these represent classes/functions are used for custom rendering of data values for data-tables or exporting data, right ? But on enabling the |
|
for req/commit there is no 'commit_id' field, there is an 'id' field which in other places would be seen as a commit_id |
yes |
|
Okay so |
|
A unit test would create a test req_commit record, and then run req_CommitRepresent against the record ID. Then check whether the return value is correct, and finally remove the test record. That's all there is to it, nothing fancy. |
|
There are over 500 examples of unit tests in modules/unit_tests and plenty of documentation for the Python unittest module. It's not rocket science - the trick is just to understand what you're trying to prove with the test and implement a valid test. |
|
And I said that with the unittest only to spare you the effort of digging for a case where it is visible, because I understood that you couldn't find one. In fact, there may not be one ;) |
No description provided.