serratus-bio / serratus.io

Front-end code for Serratus project website
https://serratus.io
GNU Affero General Public License v3.0
11 stars 11 forks source link

Palmprint/PalmID Integration into RdRP Explorer #169

Closed binoyskumar92 closed 1 year ago

binoyskumar92 commented 2 years ago

preview link

This pull request addresses issue #158

Summary of changes

Post-merge

ababaian commented 2 years ago

In the Analyze palmprint link the URL path does not parse the input sequence correctly, see: https://158-palmprint-palmid-integration-into-rdp-explorer.serratus.io/palmid?fastaInput=ERR2756788_1_u21405MGWDYPKCDRALPSMIRMISAMILGSKHVTCCTASDKYYRLCNELAQVLTEVVYSNGGFYFKPGGTTSGDATTAYANSVFNIFQAVSANINRLLTVDSYAIHNESVKSLQRQLYDNCYRATSVDATFVSDYYQFLRKHFSMMILSDDGVV

Which as a parsed input gives

>Serratus_palmid
ERRUMGWDYPKCDRALPSMIRMISAMILGSKHVTCCTASDKYYRLCNELA
QVLTEVVYSNGGFYFKPGGTTSGDATTAYANSVFNIFQAVSANINRLLTV
DSYAIHNESVKSLQRQLYDNCYRATSVDATFVSDYYQFLRKHFSMMILSD
DGVV

Where it should be

>ERR2756788_1_u21405
MGWDYPKCDRALPSMIRMISAMILGSKHVTCCTASDKYYRLCNELAQVLT
EVVYSNGGFYFKPGGTTSGDATTAYANSVFNIFQAVSANINRLLTVDSYA
IHNESVKSLQRQLYDNCYRATSVDATFVSDYYQFLRKHFSMMILSDDGVV

The corrected URL should use URL escape characters for > (%3E) and newline (%0A) like so:

https://158-palmprint-palmid-integration-into-rdp-explorer.serratus.io/palmid?fastaInput=%3EERR2756788_1_u21405%0AMGWDYPKCDRALPSMIRMISAMILGSKHVTCCTASDKYYRLCNELAQVLTEVVYSNGGFYFKPGGTTSGDATTAYANSVFNIFQAVSANINRLLTVDSYAIHNESVKSLQRQLYDNCYRATSVDATFVSDYYQFLRKHFSMMILSDDGVV

binoyskumar92 commented 2 years ago

Thanks. good catch.

binoyskumar92 commented 2 years ago

Updated

ababaian commented 2 years ago

@binoyskumar92 think you can clean this up and get it done?

I forgot you had this almost done and I took a stab at it: https://github.com/serratus-bio/serratus.io/pull/188

One suggestion is to not use > and \n in the URL parsing, instead pass the header and the sequence as two parameters: head and seq. This simplifies dealing with the special escape characters we can use some boilerplate header if one is not included, the requirement really is a sequence. Take a look at my PR above, it's not good (bit buggy on clear), but the idea is there.

binoyskumar92 commented 2 years ago

Sure. I kinda assumed these changes were merged after 1st round of review. Did not keep track of second round of review from victor.