Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Paste code to submit feature #2720

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions etc/db-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,14 @@
- category: Display
description: Options related to the DOMjudge user interface.
items:
- name: default_submission_code_mode
meisterT marked this conversation as resolved.
Show resolved Hide resolved
type: int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this an array_val, then one can select which ones are enabled. By default let's enable upload for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but in that case, the cookies will also need a major revamp.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should prevent the array from being empty here. How should I handle it when someone submits the form with an empty array?

Should I use an alert?

I haven’t seen any other similar menus to refer to for this implementation.

Sorry about that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would check that the array doesn't get empty and if it does refuse to save or use a danger flash with the message. Alternative is to have a message in the config-check page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think implementing the required restriction in the backend can be done in the next PR, as it's more general. Including it in this commit would make things confusing.

can i do it this way?

default_value: 0
public: true
description: Select the default submission method for the team
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these the default or allowed submission methods? Also, it would be good to clarify that these are submission methods from the web interface, as there's also the submit client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowed submission methods, with the default set to upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is submit client?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as6325400 marked this conversation as resolved.
Show resolved Hide resolved
options:
0: Paste
1: Upload
- name: output_display_limit
type: int
default_value: 2000
Expand Down
22 changes: 22 additions & 0 deletions webapp/public/style_domjudge.css
Original file line number Diff line number Diff line change
Expand Up @@ -699,3 +699,25 @@ blockquote {
padding: 3px;
border-radius: 5px;
}

#submissionTabs.container {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
gap: 10px;
}

.editor-container {
border: 1px solid #ddd;
border-radius: 4px;
box-shadow: 0 2px 5px rgba(0, 0, 0, 0.1);
padding: 10px;
margin-top: 10px;
margin-bottom: 10px;
background-color: #fafafa;
max-height: 400px;
overflow: auto;
}

.editor-container:hover {
box-shadow: 0 4px 10px rgba(0, 0, 0, 0.2);
}
115 changes: 90 additions & 25 deletions webapp/src/Controller/Team/SubmissionController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use App\Entity\Submission;
use App\Entity\Testcase;
use App\Form\Type\SubmitProblemType;
use App\Form\Type\SubmitProblemPasteType;
use App\Service\ConfigurationService;
use App\Service\DOMJudgeService;
use App\Service\EventLogService;
Expand Down Expand Up @@ -60,47 +61,111 @@ public function createAction(Request $request, ?Problem $problem = null): Respon
if ($problem !== null) {
$data['problem'] = $problem;
}
$form = $this->formFactory
$formUpload = $this->formFactory
->createBuilder(SubmitProblemType::class, $data)
->setAction($this->generateUrl('team_submit'))
->getForm();

$form->handleRequest($request);
$formPaste = $this->formFactory
->createBuilder(SubmitProblemPasteType::class, $data)
->setAction($this->generateUrl('team_submit'))
->getForm();

if ($form->isSubmitted() && $form->isValid()) {
$formUpload->handleRequest($request);
$formPaste->handleRequest($request);
if ($formUpload->isSubmitted() || $formPaste->isSubmitted()) {
if ($contest === null) {
$this->addFlash('danger', 'No active contest');
} elseif (!$this->dj->checkrole('jury') && !$contest->getFreezeData()->started()) {
$this->addFlash('danger', 'Contest has not yet started');
} else {
/** @var Problem $problem */
$problem = $form->get('problem')->getData();
/** @var Language $language */
$language = $form->get('language')->getData();
/** @var UploadedFile[] $files */
$files = $form->get('code')->getData();
if (!is_array($files)) {
$files = [$files];
$problem = null;
$language = null;
$files = [];
$entryPoint = null;
$message = '';

if ($formUpload->isSubmitted() && $formUpload->isValid()) {
$problem = $formUpload->get('problem')->getData();
$language = $formUpload->get('language')->getData();
$files = $formUpload->get('code')->getData();
if (!is_array($files)) {
$files = [$files];
}
$entryPoint = $formUpload->get('entry_point')->getData() ?: null;
} elseif ($formPaste->isSubmitted() && $formPaste->isValid()) {
$problem = $formPaste->get('problem')->getData();
$language = $formPaste->get('language')->getData();
$codeContent = $formPaste->get('code_content')->getData();

if ($codeContent == null || empty(trim($codeContent))) {
$this->addFlash('danger', 'No code content provided.');
return $this->redirectToRoute('team_index');
}

$tempDir = sys_get_temp_dir();
$tempFileName = sprintf(
'submission_%s_%s_%s.%s',
$user->getUsername(),
$problem->getName(),
date('Y-m-d_H-i-s'),
$language->getExtensions()[0]
);
$tempFileName = preg_replace('/[^a-zA-Z0-9_.-]/', '_', $tempFileName);
$tempFilePath = $tempDir . DIRECTORY_SEPARATOR . $tempFileName;
file_put_contents($tempFilePath, $codeContent);

$uploadedFile = new UploadedFile(
$tempFilePath,
$tempFileName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use a simpler name here (and then of course use that as base for the entry point calculation). A nice simple naming scheme could be the shortName of the contestProblem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still want the pasted submission to have a special identifier in the filename, to differentiate it from uploads. Is it possible to do this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason the way of submitting is important? You could test with adding fileupload- to the name? You would need to test yourself if this still works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it "fileupload"? I thought it would be something like "Paste."

However, I now think using "shortName" directly is fine too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you think the short name as given by the contest problem is ok, let's use that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the short name allows various characters. I will still use preg_replace('/[^a-zA-Z0-9_.-]/', '_', $tempFileName); to handle file naming using the short name.

'application/octet-stream',
null,
true
);

$files = [$uploadedFile];
$entryPoint = $tempFileName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered ignoring the entry in Paste mode, since only one file can be uploaded with Paste?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One example is Java, where the entry point basically specifies the class. If you would make the name of the file clear in the UI (or even an input that can be changed after selecting the problem) we could do the same auto-detection (with possibility to change it) with the entry point as for the uploaded file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's feasible in terms of implementation, but Paste code is essentially designed for convenience with a single file. If we allow changes to the file name, it somewhat defeats the original purpose of this feature. Alternatively, we could try handling languages where the entry point is not the file name with special treatment? (Actually, this seems similar to the original approach for uploads.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I'm currently teaching a data structures class where only C language is allowed, and the usage of Paste is significantly higher than that of upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I followed that approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, I'd like to ask how I can reference code in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything else to adjust?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
$entryPoint = $form->get('entry_point')->getData() ?: null;
$submission = $this->submissionService->submitSolution(
$team, $this->dj->getUser(), $problem->getProbid(), $contest, $language, $files, 'team page', null,
null, $entryPoint, null, null, $message
);

if ($submission) {
$this->addFlash(
'success',
'Submission done! Watch for the verdict in the list below.'

if ($problem && $language && !empty($files)) {
$submission = $this->submissionService->submitSolution(
$team,
$this->dj->getUser(),
$problem->getProbid(),
$contest,
$language,
$files,
'team page',
null,
null,
$entryPoint,
null,
null,
$message
);
} else {
$this->addFlash('danger', $message);

if ($submission) {
$this->addFlash('success', 'Submission done! Watch for the verdict in the list below.');
} else {
$this->addFlash('danger', $message);
}

return $this->redirectToRoute('team_index');
}
return $this->redirectToRoute('team_index');
}
}

$active_tab = (bool) $this->config->get('default_submission_code_mode') == 0 ? 'paste' : 'upload';
if ($this->dj->getCookie('active_tab') != null) {
$active_tab = $this->dj->getCookie('active_tab');
}

$data = ['form' => $form->createView(), 'problem' => $problem];
$data = [
'formupload' => $formUpload->createView(),
'formpaste' => $formPaste->createView(),
'active_tab' => $active_tab,
'problem' => $problem,
];
$data['validFilenameRegex'] = SubmissionService::FILENAME_REGEX;

if ($request->isXmlHttpRequest()) {
Expand Down
81 changes: 81 additions & 0 deletions webapp/src/Form/Type/SubmitProblemPasteType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php declare(strict_types=1);

namespace App\Form\Type;

use App\Entity\Language;
use App\Entity\Problem;
use App\Service\ConfigurationService;
use App\Service\DOMJudgeService;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\Query\Expr\Join;
use Symfony\Bridge\Doctrine\Form\Type\EntityType;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\TextareaType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\FormEvents;
use Symfony\Component\Validator\Constraints\Callback;
use Symfony\Component\Validator\Context\ExecutionContextInterface;
use Symfony\Component\Form\Extension\Core\Type\HiddenType;

class SubmitProblemPasteType extends AbstractType
{
public function __construct(
protected readonly DOMJudgeService $dj,
protected readonly ConfigurationService $config,
protected readonly EntityManagerInterface $em
) {
}

public function buildForm(FormBuilderInterface $builder, array $options): void
{
$user = $this->dj->getUser();
$contest = $this->dj->getCurrentContest($user->getTeam()->getTeamid());

$builder->add('code_content', HiddenType::class, [
'required' => true,
]);
$problemConfig = [
'class' => Problem::class,
'query_builder' => fn(EntityRepository $er) => $er->createQueryBuilder('p')
->join('p.contest_problems', 'cp', Join::WITH, 'cp.contest = :contest')
->select('p', 'cp')
->andWhere('cp.allowSubmit = 1')
->setParameter('contest', $contest)
->addOrderBy('cp.shortname'),
'choice_label' => fn(Problem $problem) => sprintf(
'%s - %s',
$problem->getContestProblems()->first()->getShortName(),
$problem->getName()
),
'placeholder' => 'Select a problem',
];
$builder->add('problem', EntityType::class, $problemConfig);

$builder->add('language', EntityType::class, [
'class' => Language::class,
'query_builder' => fn(EntityRepository $er) => $er
->createQueryBuilder('l')
->andWhere('l.allowSubmit = 1'),
'choice_label' => 'name',
'placeholder' => 'Select a language',
]);

$builder->add('entry_point', TextType::class, [
'label' => 'Entry point',
'required' => false,
'help' => 'The entry point for your code.',
'row_attr' => ['data-entry-point' => '']
]);
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) use ($problemConfig) {
$data = $event->getData();
if (isset($data['problem'])) {
$problemConfig += ['row_attr' => ['class' => 'd-none']];
$event->getForm()->add('problem', EntityType::class, $problemConfig);
}
});
}
}
2 changes: 2 additions & 0 deletions webapp/templates/base.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
<script src="{{ asset("js/bootstrap.bundle.min.js") }}"></script>

<script src="{{ asset("js/domjudge.js") }}"></script>
<script src="{{ asset('js/ace/ace.js') }}"></script>
as6325400 marked this conversation as resolved.
Show resolved Hide resolved

{% for file in customAssetFiles('js') %}
<script src="{{ asset('js/custom/' ~ file) }}"></script>
{% endfor %}
Expand Down
12 changes: 6 additions & 6 deletions webapp/templates/team/submit.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@
</div>
{% else %}

{{ form_start(form) }}
{{ form_row(form.code) }}
{{ form_row(form.problem) }}
{{ form_row(form.language) }}
{{ form_row(form.entry_point) }}
{{ form_start(formupload) }}
{{ form_row(formupload.code) }}
{{ form_row(formupload.problem) }}
{{ form_row(formupload.language) }}
{{ form_row(formupload.entry_point) }}
<div class="mb-3">
<button type="submit" class="btn-success btn"><i class="fas fa-cloud-upload-alt"></i> Submit
</button>
</div>
{{ form_end(form) }}
{{ form_end(formupload) }}

{% endif %}
{% endif %}
Expand Down
64 changes: 51 additions & 13 deletions webapp/templates/team/submit_modal.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,59 @@
{% include 'partials/alert.html.twig' with {'type': 'danger', 'message': 'Submissions (temporarily) disabled.'} %}
</div>
{% else %}
{{ form_start(form) }}
<div class="modal-body">
{{ form_row(form.code) }}
<div class="alert d-none" id="files_selected"></div>
{{ form_row(form.problem) }}
{{ form_row(form.language) }}
{{ form_row(form.entry_point) }}
</div>
<div class="modal-footer">
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>
<button type="submit" class="btn-success btn">
<i class="fas fa-cloud-upload-alt"></i> Submit
</button>
<ul class="nav nav-tabs container text-center" id="submissionTabs" role="tablist" style="width: 100%">
<li class="nav-item" role="presentation" >
<a class="nav-link {% if active_tab == 'upload' %}active{% endif %}" id="upload-tab" data-bs-toggle="tab" href="#upload" role="tab" aria-controls="upload" aria-selected="{% if active_tab == 'upload' %}true{% else %}false{% endif %}" onclick="setCookie('active_tab', 'upload')" >Upload File</a>
</li>
<li class="nav-item text-center" role="presentation">
<a class="nav-link {% if active_tab == 'paste' %}active{% endif %}" id="paste-tab" data-bs-toggle="tab" href="#paste" role="tab" aria-controls="paste" aria-selected="{% if active_tab == 'paste' %}true{% else %}false{% endif %}" onclick="setCookie('active_tab', 'paste')">Paste Code</a>
</li>
</ul>
<div class="tab-content" id="submissionTabsContent" style="margin-top: 20px;">
<div class="tab-pane fade {% if active_tab == 'upload' %}show active{% endif %}" id="upload" role="tabpanel" aria-labelledby="upload-tab">
{{ form_start(formupload) }}
{{ form_row(formupload.code) }}
<div class="alert d-none" id="files_selected"></div>
{{ form_row(formupload.problem) }}
{{ form_row(formupload.language) }}
{{ form_row(formupload.entry_point) }}
<div class="modal-footer">
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>
<button type="submit" class="btn btn-success">
<i class="fas fa-cloud-upload-alt"></i> Submit Upload
</button>
</div>
{{ form_end(formupload) }}
</div>

<div class="tab-pane fade {% if active_tab == 'paste' %}show active{% endif %}" id="paste" role="tabpanel" aria-labelledby="paste-tab">
{{ form_start(formpaste) }}
{{ form_widget(formpaste.code_content) }}
<label for="codeInput">Paste your code here:</label>
<div class="editor-container">
{{ "" | codeEditor(
"_team_submission_code",
"c_cpp",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of moving the language selection above the editor? This way you can also use it here to fix syntax highlighting in the editor (as idea).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea. When I first implemented it, I wasn't sure how to dynamically adjust the syntax highlighting in the editor by changing the language in the menu. Is that achievable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that depends on the twig template.

Let's leave that for a follow up PR in that case.

true,
formpaste.code_content.vars.id,
null,
formpaste.language.vars.value
) }}
</div>
{{ form_row(formpaste.problem) }}
{{ form_row(formpaste.language) }}
{{ form_row(formpaste.entry_point) }}
<div class="modal-footer">
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>
<button type="submit" class="btn btn-primary">
<i class="fas fa-paste"></i> Submit Paste
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's call the button just Submit, same with the other button

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both "Upload" and "Paste" are available at the same time, would it be weird or cause confusion if both buttons are labeled "Submit"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is confusing as in both cases you are submitting what you filled in in the form you are currently seeing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now, I misunderstood what you meant.

</button>
</div>
{{ form_end(formpaste) }}
</div>
</div>
</div>
{{ form_end(form) }}
{% endif %}
</div>
</div>
Expand Down
Loading