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

Fix/pisa25 472/id attribute removed #2388

Merged
merged 18 commits into from
Feb 8, 2024

Conversation

shaveko
Copy link
Contributor

@shaveko shaveko commented Aug 25, 2023

related to https://oat-sa.atlassian.net/browse/PISA25-472

Description

This PR adds comments to generated code to warn author if he uses sourceDialog to edit code of the interaction

How to test

Can be validated on kitchen env

Create item with widget, i.e. img (block or inline). Check the source code - the widget code should be wrapped with warning comments

<!-- autogenerated code, do not edit -->
widget code
<!-- end of autogenerated code -->

when item is saved, saveItem should not contain these comments

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (777875f) 16.06% compared to head (c98a697) 16.06%.
Report is 1 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #2388   +/-   ##
==========================================
  Coverage      16.06%   16.06%           
  Complexity      4207     4207           
==========================================
  Files            391      391           
  Lines          13491    13491           
==========================================
  Hits            2168     2168           
  Misses         11323    11323           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shaveko shaveko requested a review from jsconan August 28, 2023 17:13
Copy link
Contributor

@jsconan jsconan left a comment

Choose a reason for hiding this comment

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

It works, but the comments remain in the final code, is it expected?

I also a few comments regarding code style

views/js/qtiCreator/widgets/static/helpers/comments.js Outdated Show resolved Hide resolved
views/js/qtiCreator/widgets/static/helpers/comments.js Outdated Show resolved Hide resolved
views/js/qtiCreator/widgets/static/helpers/comments.js Outdated Show resolved Hide resolved
views/js/qtiCreator/widgets/static/helpers/comments.js Outdated Show resolved Hide resolved
views/js/qtiCreator/widgets/static/helpers/widget.js Outdated Show resolved Hide resolved
views/js/qtiXmlRenderer/renderers/Container.js Outdated Show resolved Hide resolved
@shaveko shaveko requested a review from jsconan August 29, 2023 17:10
Copy link
Contributor

@jsconan jsconan left a comment

Choose a reason for hiding this comment

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

It works very well, a comment is added before entering the edit mode.
The comment is properly removed once the edition is finished, and it is not present in the saved data. Good job!

Note: since the code was modified since the last bundling, you may need to bundle it again before pushing the code to a testing env 😉

Review Checklist

  • New code is covered by tests (if applicable)
  • Tests are running successfully (old and new ones) on my local machine (if applicable)
  • New code respects code style rules
  • New code respects best practices
  • New code is not subject to concurrency issues (if applicable)
  • Feature is working correctly on my local machine (if applicable)
  • Acceptance criteria are respected
  • Pull request title and description are meaningful
  • Pull request's target is not master
  • Commits are following conventional commits
  • Commits messages are meaningful
  • Commits are atomic

Copy link

github-actions bot commented Feb 8, 2024

Version

Target Version 30.5.8
Last version 30.5.7

There are 0 BREAKING CHANGE, 0 feature, 7 fixes

@shaveko shaveko merged commit 6a327f3 into develop Feb 8, 2024
3 of 5 checks passed
@shaveko shaveko deleted the fix/PISA25-472/id-attribute-removed branch February 8, 2024 11:25
@oat-sa oat-sa deleted a comment from swiec Feb 8, 2024
@shaveko shaveko restored the fix/PISA25-472/id-attribute-removed branch February 12, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants