String PID support #511

undefined-moe · 2019-05-11T04:47:30Z

For #510
There's maybe some bug.
Still testing.

…tid}/edit

iceboy233 · 2019-05-11T12:34:06Z

vj4/handler/homework.py

@@ -157,7 +157,7 @@ class HomeworkCodeHandler(base.OperationHandler):
                      file_name='{}.zip'.format(tdoc['title']))


-@app.route('/homework/{tid}/{pid:-?\d+|\w{24}}', 'homework_detail_problem')
+@app.route('/homework/{tid}/{pid:[A-Z0-9]+}', 'homework_detail_problem')


Why not allowing lowercase letters and maybe other characters?

It creates conflict.

What conflict? Can you explain more?

not this route
my fault.

this one.

Emmm. Then I think we should not simply support string PIDs without changing the URL scheme. I think depending on URL case sensitivity is not a good idea.

By the way, it would be better to text instead of screenshot for code snippets. It is more friendly for more types of browsers/screen readers. For example:

@app.route(...) class XxxHandler: pass

Some user may just collected a URL in his browser.
Changing the URL scheme will make then confused as it throws 404 error.

Of course we should change the URL in a backward compatible way, and preferably still generate the same URL for numeric/objectid PIDs. This can be achieved by making multiple routes to point to the same handler.

One proposal is to use (optional) named parameter in URL. For example: /homework/tid=xxx/pid=xxx/scoreboard. We can also use query strings.

What do you think?

Scoreboard for each problem?
so weird...

My bad. The example should be /homework/tid=xxx/pid=xxx/submit. It is just an example.

the submit handler can work properly.
but handlers like 'homework_edit' 'homework_scoreboard' will be handled by 'homework_problem' handler if we enables lowercase pid.

iceboy233 · 2019-05-11T12:35:40Z

vj4/ui/templates/components/problem.html

@@ -10,7 +10,7 @@
  {%- endif %}
  >
  {%- endif %}
-    {% if pdoc['doc_id']|string|length < 10 %}P{{ pdoc['doc_id'] }} {% endif %}{{ pdoc['title'] }}
+    {% if pdoc['doc_id'] is number %}P {{ pdoc['doc_id'] }}{% elif pdoc['doc_id']|string|length < 10 %} {{ pdoc['doc_id'] }} {% endif %}{{ pdoc['title'] }}


The logic here seems complicated and incorrect. Can we implement them at Python side?

Then we have to make a new template but I think it's not necessary.

Actually the logic is:
If it is a number,we add a 'p' before.
Else if it's not an object id, show the specified string PID.
Otherwise, no ID will be shown.

You added a space between P and the numeric doc_id.

I'm not sure what number is in jinja2 but I suspect it includes floating point number which should not be included.

Why is there a magic number 10?

Again you added some random spaces in the string doc_id rendering.

Why is not displaying doc_id preferred in some situations?

Oh..I added this space by mistake,I'll fix later.
If we always show doc_id,It will show pid 'A123' as 'PA12'

Acturally I don't wrote the number 10.
It's in older version.

Now it looks like this:

twd2

What should I do if I want an ObjectId as PID?

vj4/locale/zh_CN.yaml

undefined-moe · 2019-05-11T17:26:54Z

Why the object id is needed?

twd2 · 2019-05-11T18:27:10Z

Some users may need that... This is back-compatibility.

twd2 · 2019-05-11T18:40:47Z

Can we write a regular expression to match the strings other than some meaningful words like scoreboard, create or copy...?

undefined-moe · 2019-05-11T18:45:13Z

if a user want's to create a problem called 'copy'...

undefined-moe · 2019-05-11T18:46:20Z

And I don't understand why objectid is better than numid for user.

twd2 · 2019-05-11T18:52:42Z

It is the default behavior to allocate ObjectIds as PIDs. And I don't think we should change the default behavior.

undefined-moe · 2019-05-12T01:47:20Z

Why shouldn't?
Some new users upload a problem,and found it has such a long objectid and hard to remember.
The system now doesn't allow users to change pid or just delete the problem.

undefined-moe · 2019-05-12T10:43:27Z

It seems that most users want's number PIDs instead of Object ID.

twd2

Why are so many lines changed? Could you please do not change the order of the handlers?

twd2 · 2019-05-13T10:02:54Z

The new URL schema is under discussion.

undefined-moe · 2019-05-13T12:07:46Z

Replaced by #514
Closed.

root and others added 4 commits May 11, 2019 12:45

String PID support

862c1cb

Fix templates

f0e4a9e

Add a space

70f5475

contest

8434471

undefined-moe marked this pull request as ready for review May 11, 2019 08:47

twd2 requested review from iceboy233, twd2 and moesoha and removed request for iceboy233 and twd2 May 11, 2019 08:56

Remove lowercase PID support as it will cause an error for /contest/{…

46a5ad0

…tid}/edit

iceboy233 reviewed May 11, 2019

View reviewed changes

undefined-moe added 4 commits May 11, 2019 20:53

Fix template

e554066

Fix support for _id

37137fd

Allow users add String PID problems.

57d5312

Add validator

ac2af5b

twd2 reviewed May 11, 2019

View reviewed changes

vj4/locale/zh_CN.yaml Outdated Show resolved Hide resolved

Change limit to 24

89aecbc

undefined-moe added 2 commits May 12, 2019 13:37

Update translation

5b8e8a2

Add lowercase support

9cefd5f

-

052ca0f

twd2 reviewed May 13, 2019

View reviewed changes

twd2 added the need-discussion label May 13, 2019

undefined-moe closed this May 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

String PID support #511

String PID support #511

undefined-moe commented May 11, 2019 •

edited

Loading

iceboy233 May 11, 2019

undefined-moe May 11, 2019

iceboy233 May 11, 2019

undefined-moe May 11, 2019

iceboy233 May 11, 2019

undefined-moe May 11, 2019

iceboy233 May 11, 2019

undefined-moe May 11, 2019

iceboy233 May 11, 2019

undefined-moe May 11, 2019

iceboy233 May 11, 2019

undefined-moe May 11, 2019

undefined-moe May 11, 2019

iceboy233 May 11, 2019 •

edited

Loading

undefined-moe May 11, 2019

undefined-moe May 11, 2019

undefined-moe May 11, 2019

twd2 left a comment

undefined-moe commented May 11, 2019

twd2 commented May 11, 2019

twd2 commented May 11, 2019

undefined-moe commented May 11, 2019

undefined-moe commented May 11, 2019

twd2 commented May 11, 2019 •

edited

Loading

undefined-moe commented May 12, 2019

undefined-moe commented May 12, 2019

twd2 left a comment

twd2 commented May 13, 2019

undefined-moe commented May 13, 2019

String PID support #511

String PID support #511

Conversation

undefined-moe commented May 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iceboy233 May 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twd2 left a comment

Choose a reason for hiding this comment

undefined-moe commented May 11, 2019

twd2 commented May 11, 2019

twd2 commented May 11, 2019

undefined-moe commented May 11, 2019

undefined-moe commented May 11, 2019

twd2 commented May 11, 2019 • edited Loading

undefined-moe commented May 12, 2019

undefined-moe commented May 12, 2019

twd2 left a comment

Choose a reason for hiding this comment

twd2 commented May 13, 2019

undefined-moe commented May 13, 2019

undefined-moe commented May 11, 2019 •

edited

Loading

iceboy233 May 11, 2019 •

edited

Loading

twd2 commented May 11, 2019 •

edited

Loading