JM - (SPARCRequest & SPARCDashboard) Step 2 Edit Study RMID Bugs [#150810679] #1123
Conversation
jwiel86
force-pushed
the
jm-rmid-bug
branch
4 times, most recently
from
6c7b210
to
5687d56
Compare
| @@ -46,3 +46,6 @@ textarea | |||
|
|
|||
| .gray_note | |||
| color: gray | |||
|
|
|||
| .red_exclamation | |||
There was a problem hiding this comment.
The text-danger class is designed to do this using the SPARC color scheme red
| @@ -97,6 +97,8 @@ def new | |||
| session[:protocol_type] = params[:protocol_type] | |||
| gon.rm_id_api_url = RESEARCH_MASTER_API | |||
| gon.rm_id_api_token = RMID_API_TOKEN | |||
| @rmid_server_down = @protocol.rmid_server_status | |||
There was a problem hiding this comment.
Since this is repeated multiple times, it might be worth putting inside a helper method in Dashboard::BaseController
| .col-lg-3 | ||
| = form.text_field :research_master_id, class: 'form-control research-master-field', disabled: rmid_server_down | ||
| - if rmid_server_down | ||
| .glyphicon.glyphicon-exclamation-sign.red_exclamation{ title: t(:protocols)[:summary][:tooltips][:rmid_server_down], data: { toggle: 'tooltip', delay: '{"show":"500"}' } } |
There was a problem hiding this comment.
text-danger could replace red_exclamation here
| @@ -18,7 +18,7 @@ | |||
| -# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR~ | |||
| -# TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.~ | |||
| #flashes_container | |||
| - if !content_for?(:error_messages) | |||
| - if @rmid_server_down | |||
There was a problem hiding this comment.
This seems like a very niche change. The only change is removing the = content_for(:error_messages) inside of the flash. @jwiel86 can you send me a slack message or just reply here why you needed to remove it for this one case?
There was a problem hiding this comment.
Approving this comment for now. Discussed with @jwiel86 and we decided that it would seem to work removing the if-else altogether, but to be safe for now we should leave it like this and revisit it later.
jwiel86
force-pushed
the
jm-rmid-bug
branch
3 times, most recently
from
551b027
to
3ba1422
Compare
| @@ -30,10 +30,12 @@ | |||
| = form.label :research_master_id, class: 'col-lg-2 control-label rm-id' do | |||
| = link_to t(:protocols)[:studies][:information][:research_master_id], RESEARCH_MASTER_LINK, target: '_blank' | |||
| .col-lg-3 | |||
| = form.text_field :research_master_id, class: 'form-control research-master-field', readonly: action_name == "edit" && admin | |||
| - if action_name == "edit" && admin | |||
| = form.text_field :research_master_id, class: 'form-control research-master-field', readonly: action_name == "edit" && admin, disabled: rmid_server_down | |||
There was a problem hiding this comment.
As discussed with @jwiel86 on Slack, there is a bug where if you press "Save" on a new Protocol when there are errors and when the server is down, this field is no longer disabled.
There was a problem hiding this comment.
@kyle-glick This bug has now been fixed, see app/views/protocols/create.js.coffee and app/views/dashboard/protocols/create.js.coffee. Thank you for manually testing!
jwiel86
force-pushed
the
jm-rmid-bug
branch
3 times, most recently
from
8aa88c8
to
d051b42
Compare
jwiel86
force-pushed
the
jm-rmid-bug
branch
from
d051b42
to
b7e4a5e
Compare
https://www.pivotaltracker.com/story/show/150810679