Teammates: Feedback Session: Remove 'Never Publish' option

[sukanta-27]

Environment:
master branch.
Don't know if this is a bug or intentional. So, went along with the bug template.
Steps to reproduce:

• Login as Instructor
• Create a new session, set a Opening time so earlier than current time, to get a Open session
• Set the option Responses visible from to Never.
• Now go back to Home or Sessions, and find the session you created in the session table
• Open the drop down button in Results action, or go to Results page and click Publish Results

Expected behaviour: There should be a error message that the session can not be published as the session is set to never get published. Or, the Publish Results button should be disabled.

Actual behaviour: The session gets successfully published.

I think the proper steps to publish results for these kind of sessions should be, Edit -> Responses visible from -> Any option other than Never -> Save Changes.

[whipermr5]

This was an unintentional side effect of #8094 (see https://github.com/TEAMMATES/teammates/pull/8408#issuecomment-364468574). Anyway, private sessions are about to be removed: #8447.

[sukanta-27]

@whipermr5 Need to clarify two things.
I checked out commits before #8094, and this issue seems to persist. sessions that are not private can be published even though the Response visible from to Never!
How is this related to private sessions? (Is the Never option mentioned above only applicable to Private sessions?)
And if that is the case, should I remove the Response visitble from->Never option too, (Screenshot of the current changes are shown in : https://github.com/TEAMMATES/teammates/issues/8447#issuecomment-366473655)?

[wkurniawan07]

@sukanta-27, that option only guarantees that the session will not be automatically published after a certain timestamp. Nothing stops anyone with sufficient permission to manually publish the session at any time.

[sukanta-27]

@wkurniawan07 But, can't that be accomplished by the option Publish manually as well? Then, what is the use point of having this option?

[damithc]

But, can't that be accomplished by the option Publish manually as well? Then, what is the use point of having this option?

Technically, yes. I guess we decided to have the 'never' option to better match user intentions. It's a bit unintuitive for a user who doesn't intend to publish at all to choose 'publish manually'.

[sukanta-27]

@damithc So Sir, can we implement a feature where, if a user tries to publish the session that has been set to never get published, he/she will receive an error (Like Can not publish a session that is set to never publish, change the 'Response Visible Time' to a option other than 'Never' to publish this session), or a warning message (Like The session is set to never get published, are you sure you want to publish it?), so that a user who does not want a particular session to get published, doesn't accidentally publish the session!

[damithc]

@damithc So Sir, can we implement a feature where, if a user tries to publish the session that has been set to never get published, he/she will receive an error (Like Can not publish a session that is set to never publish, change the 'Response Visible Time' to a option other than 'Never' to publish this session), or a warning message (Like The session is set to never get published, are you sure you want to publish it?), so that a user who does not want a particular session to get published, doesn't accidentally publish the session!

Yes, that can be useful to prevent accidental publishing, as long as it can be implemented easily. We don't want to increase code base too much as a result of adding features that we can live without.

[sukanta-27]

Yes, that can be useful to prevent accidental publishing, as long as it can be implemented easily. We don't want to increase code base too much as a result of adding features that we can live without.

@damithc I haven't started working on it yet, so don't know about the implementation difficulty as of yet. But, can I work on this and see how this turns out?

[sukanta-27]

@damithc Instead of going with the warning message I guess it would be better to go with the error message, as warning messages can be easily ignored (clicking the OK button without looking into the message, specially for those who are using the system for a while). For the UI change, can we use something like this? (If yes then I can submit a PR for further review)

Screenshot:

[whipermr5]

@whipermr5 Need to clarify two things.
I checked out commits before #8094, and this issue seems to persist. sessions that are not private can be published even though the Response visible from to Never!
How is this related to private sessions? (Is the Never option mentioned above only applicable to Private sessions?)
And if that is the case, should I remove the Response visitble from->Never option too, (Screenshot of the current changes are shown in : #8447 (comment))?

Oops I mixed this up with Session visible from; my bad 😅

@damithc So Sir, can we implement a feature where, if a user tries to publish the session that has been set to never get published, he/she will receive an error (Like Can not publish a session that is set to never publish, change the 'Response Visible Time' to a option other than 'Never' to publish this session), or a warning message (Like The session is set to never get published, are you sure you want to publish it?), so that a user who does not want a particular session to get published, doesn't accidentally publish the session!

I feel the warning option is better. The error adds an additional step for users who actually want something published but selected the wrong option when creating the session or simply changed their mind after creating the session. If they dismiss the warning, it is reasonable to assume that they read it. They can always unpublish the session if they made a mistake.

[damithc]

They can always unpublish the session if they made a mistake.

Given that, perhaps we just leave it as is? If we add a warning/error here, in addition to having more code to maintain, we add time to tests which is a permanent cost.

[whipermr5]

Then I'd suggest merging the Never and Publish Manually option into one, e.g. Never (or publish manually).

[whipermr5]

On second thoughts, I feel that deleting the Publish Manually option and changing the heading from Responses visible from to Automatically publish responses would be better:

[whipermr5]

In a similar vein, the Session visible from heading can be improved to Make session visible:

This reads much better IMHO.

[sukanta-27]

@whipermr5 I think this will be better, as Automatically publish responses seems more accurate in this context. Same goes for Make session visible. If asked, I would love to do this. :smile:

[damithc]

UI design is not an exact science; hard to get right or even agree on the best design.

I'm a bit vary of designing the UI to match how the app processes things rather than how the user thinks about things. As a user I'm thinking "I'll publish the results at a later time". It's an additional step for me to also figure out "hey, that's the same as choosing 'Never' now and clicking the 'Publish' button later" (or something of the sort). i.e., the closer we can get to user intention, the easier it feels to use.

I'm also wary of combining things just because they are processed the same way. I use a banking app that has an option Transfer now / SLIIP or something like that. Although I understand it means one or the other, I always hesitate a moment before choosing that option because I have no idea what SLIIP is; I feel a bit annoyed to even having to read that word with which I don't have any business. This kind of thing (e.g., choosing an option that has some unrelated thing in it) adds anxiety to users, especially non-techie users, and makes the app less user-friendly, I think.

Of course, we also have to factor in complexity of implementation. At times we may have to choose the design that simplifies the code rather than user experience.

[sukanta-27]

I agree with what Prof. @damithc said, that users may get a little confused if they see the word Automatically and no Publish manually option.
I can suggest a few ways to deal with this issue :

1. Leave everything as it is. But with a little modification. (from followed by At sounds a bit weird IMO)
   
2. Merge two options with something like Never (Publish later) or Never (Publish manually), suggested by @whipermr5 in https://github.com/TEAMMATES/teammates/issues/8468#issuecomment-366640530
   (Something like this)
   

1. Remove the option Never completely. As the tooltip for the option Never and the function doesn't match at all. And what can be done by Never, can also be done by Publish manually.
   

And after removing private session in #8447 The option Session visible time -> Never will also be removed. see : https://github.com/TEAMMATES/teammates/issues/8447#issuecomment-366473655

@whipermr5 @damithc waiting for your view on this.

[damithc]

Option 2 reads better, but option 3 is simpler to read. @whipermr5, what do you think?

[sukanta-27]

@whipermr5 Waiting for your view on https://github.com/TEAMMATES/teammates/issues/8468#issuecomment-366681607

[whipermr5]

What about option 3 but change Publish manually to Next time so it reads better?

[sukanta-27]

@whipermr5 I think it has the possibility to confuse a lot of users. I mean Next time doesn't imply that the user will have to publish the session manually later? Users generally set this option while creating a new session, or editing it. Isn't it possible that the user will get confused by reading the term Next time, and may think he/she may has to edit the session in a later stage to set up a publish time (or something like that)?

[whipermr5]

What about Not now (publish manually) or just Not now with Publish manually as a tooltip?

[sukanta-27]

@whipermr5 I liked the Not now with Publish manually as a tooltip option. Not now (publish manually) is a little verbose for a radio button title. Isn't it?

[damithc]

@whipermr5 I liked the Not now with Publish manually as a tooltip option. Not now (publish manually) is a little verbose for a radio button title. Isn't it?

I think we should have the Publish manually as the main text. It's too important to relegate to the tool tip.

[sukanta-27]

@damithc Okay, sir. I will make the changes accordingly.

[sukanta-27]

@damithc Sir, just to be clear, you are suggesting not to change the Publish manually text at all, right?
Or, are you suggesting we should keep the word Not now (Publish manually) [Therefore not to completely remove the word publish manually]?

[damithc]

Let's try Not now (Publish manually)