Fixed issue:Journal Entries can be renamed to blank via DetailView

[chimosky]

@srevinsaju @quozl can you test?

This was moved from #663.

The comment made by @i5o here https://github.com/sugarlabs/sugar/pull/663#issuecomment-260526997 isn't a blocker as the entry doesn't handle when the enter key is pressed and I don't think it needs to, so pressing the enter key would have no effect.

[quozl]

Thanks for asking. I'm not in favour of the change. My opinions are in #663, bug 4940 and #752. I would accept the change if;

• the entry title is not restored to previous value,
• the message title cannot be empty is written as title is usually not left empty.

[srevinsaju]

I restarted bugs.sugarlabs.org back, but I forgot to restore the domain names back. Will review this PR after getting to bug 4940

[quozl]

Thanks. My main comment was that the user experience was inconsistent between journal list and detail view. There's also the activity toolkit. Quoting from 4940 ...

My opinion is the bug seems like censorship; why can't the learner use a blank or empty name for a journal object? It is an unnecessary restriction on expression. Why can't Sugar just do what it is told? The learner can always discover how to rename again. So I'd like to see the first patch reverted, and instead patches made to the Help activity explaining the rename to blank feature.

[srevinsaju]

Restored bugs.sugarlabs.org,

The MR looks good overall, but, yea, I am not sure whats the best decision here.

[chimosky]

Thanks for asking. I'm not in favour of the change. My opinions are in #663, bug 4940 and #752. I would accept the change if;

* the entry title is not restored to previous value,

* the message _title cannot be empty_ is written as _title is usually not left empty_.

I think that the user being able to leave a journal object without any name isn't that bad but at the same time it might be nice to have a placeholder text to indicate there's no title as that might be a bit more helpful, I'll change the alert message, thanks.

@aperezbios @walterbender thoughts?

[chimosky]

@quozl could you help me test and figure this out?

Currently, renaming and activity instance to blank via the listview uses a ConfirmationAlert and works as intended, activity name changes based on user response to the alert but while in detail view it doesn't work as intended, sometimes it changes to a blank when renaming to a blank regardless of the users' response but usually the first few changes don't seem to have any effect.

[quozl]

Tested a5689c8.

If I create an activity instance with an empty title (e.g. with Calculator and set the title empty), then open the Journal list view, start editing the name, but press enter without making a change; the alert appears, and if I press Cancel the title changes to the last title on the list. Switching to Documents then back to Journal restores the title.

On the same empty title journal entry, view details, trying to change the title, the alert appears, but the Cancel button does not work, and the logs contain TypeError: _title_alert_response_cb() takes 3 positional arguments but 5 were given.

[chimosky]

Tested a5689c8.

If I create an activity instance with an empty title (e.g. with Calculator and set the title empty), then open the Journal list view, start editing the name, but press enter without making a change; the alert appears, and if I press Cancel the title changes to the last title on the list. Switching to Documents then back to Journal restores the title.

On the same empty title journal entry, view details, trying to change the title, the alert appears, but the Cancel button does not work, and the logs contain TypeError: _title_alert_response_cb() takes 3 positional arguments but 5 were given.

My bad, I've fixed it in 12a9c44.

[quozl]

Tested 12a9c44.

In detail view, pressing enter on the title does not show the alert, but tab works. Also, pressing Cancel or Ok on the alert has to be done twice before the alert is dismissed.

For an instance with non-blank title, I click for detail view, empty the title, then press the back button. The alert appears on the list view, not the detail view. Presssing Cancel did not restore the title; it had changed to empty.

Some stack traces appear in shell.log;

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/jarabe/journal/listview.py", line 374, in __buddies_set_data_cb
    buddy = tree_model.do_get_value(tree_iter, cell._model_column_index)
  File "/usr/lib/python3/dist-packages/jarabe/journal/listmodel.py", line 160, in do_get_value
    return self._cached_row[column]
IndexError: list index out of range
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/jarabe/journal/listmodel.py", line 160, in do_get_value
    return self._cached_row[column]
IndexError: list index out of range

[quozl]

Changing an entry in list view from non-empty to non-empty is ineffective; the title remains as it was.

[chimosky]

Tested 12a9c44.

In detail view, pressing enter on the title does not show the alert, but tab works. Also, pressing Cancel or Ok on the alert has to be done twice before the alert is dismissed.

Yes pressing enter doesn't work as there's no handler for that event.

For an instance with non-blank title, I click for detail view, empty the title, then press the back button. The alert appears on the list view, not the detail view. Presssing Cancel did not restore the title; it had changed to empty.

Yes it does as before now the title only gets saved when back to list view as that's the current behaviour at master, the issue I'm having is the title not changing despite Cancel being the response and so far I don't see why that's happening, added logging warnings to see what's happening and sometimes it seems like the code that sets the title runs before the _title_alert_response_cb as the warnings added there get printed after the warnings added in if self._title_changed of _update_entry

Some stack traces appear in shell.log;

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/jarabe/journal/listview.py", line 374, in __buddies_set_data_cb
    buddy = tree_model.do_get_value(tree_iter, cell._model_column_index)
  File "/usr/lib/python3/dist-packages/jarabe/journal/listmodel.py", line 160, in do_get_value
    return self._cached_row[column]
IndexError: list index out of range
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/jarabe/journal/listmodel.py", line 160, in do_get_value
    return self._cached_row[column]
IndexError: list index out of range

First time seeing this, can you remember what you'd done before this appeared?

Changing an entry in list view from non-empty to non-empty is ineffective; the title remains as it was.

Haven't experienced this and I don't see why that happens.

[chimosky]

@quozl @srevinsaju @aperezbios kindly review.