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

ketcher2 User templates #1984

Closed
wants to merge 47 commits into from
Closed

ketcher2 User templates #1984

wants to merge 47 commits into from

Conversation

haditariq
Copy link
Contributor

@haditariq haditariq commented Jun 24, 2024

This PR resolves Sub-feature (1.4) of Issue # 138:

  • User templates available to be used within Ketcher2.
  • Ketcher2 stores templates by default in browser's local storage.
  • API endpoint is added to sync localstorage with the backend in the form of attachment/files.
  • Now, user is able to add, remove, edit user template within ketcher2
  • Demo Attached:
User.template.feature.demo.1.mp4
  • rather 1-story 1-commit than sub-atomic commits

  • commit title is meaningful => git history search

  • commit description is helpful => helps the reviewer to understand the changes

  • code is up-to-date with the latest developments of the target branch (rebased to it or whatever) => ⏩-merge for linear history is favoured

  • added code is linted

  • tests are passing (at least locally): we still have some random test failure on CI. thinking of asking spec/examples.txt to be commited

  • in case the changes are visible to the end-user,  video or screenshots should be added to the PR => helps with user testing

  • testing coverage improvement is improved.

  • CHANGELOG :  add a bullet point on top (optional: reference to github issue/PR )

  • parallele PR for documentation  on docusaurus  if the feature/fix is tagged for a release

haditariq and others added 23 commits June 7, 2024 11:25
```
bin/ket2-install.sh
```

compatible version of ketcher 2 to install tracked in
.service-dependencies
Copy link

LCOV of commit ab58f8d during Continuous Integration #3017

Summary coverage rate:
  lines......: 64.1% (13691 of 21354 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

Copy link

LCOV of commit 3e59bbe during Continuous Integration #3201

Summary coverage rate:
  lines......: 64.0% (13662 of 21345 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a


# remove file from store
file_path = Rails.root.join('uploads', Rails.env, params[:path])
File.delete(file_path) if(File.exist?(file_path))

Choose a reason for hiding this comment

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

Lint/NonAtomicFileOperation: Use atomic file operation method FileUtils.rm_f.


# remove file from store
file_path = Rails.root.join('uploads', Rails.env, params[:path])
File.delete(file_path) if(File.exist?(file_path))

Choose a reason for hiding this comment

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

Layout/SpaceAroundKeyword: Space after keyword if is missing.


# remove file from store
file_path = Rails.root.join('uploads', Rails.env, params[:path])
File.delete(file_path) if(File.exist?(file_path))

Choose a reason for hiding this comment

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

Lint/NonAtomicFileOperation: Remove unnecessary existence check File.exist?.

@haditariq haditariq changed the base branch from main to ketcher2-editor July 25, 2024 14:24
requires :path, type: String, desc: 'file path of user template'
end
delete do
user_templates = current_user.profile.user_templates

Choose a reason for hiding this comment

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

Layout/IndentationWidth: Use 2 (not 0) spaces for indentation.

new_profile) || error!('profile update failed', 500)

{ status: true }
end

Choose a reason for hiding this comment

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

Layout/BlockAlignment: end at 207, 8 is not aligned with delete do at 191, 6.


# remove file from store
file_path = Rails.root.join('uploads', Rails.env, params[:path])
File.delete(file_path) if File.exist?(file_path)

Choose a reason for hiding this comment

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

Lint/NonAtomicFileOperation: Use atomic file operation method FileUtils.rm_f.


# remove file from store
file_path = Rails.root.join('uploads', Rails.env, params[:path])
File.delete(file_path) if File.exist?(file_path)

Choose a reason for hiding this comment

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

Lint/NonAtomicFileOperation: Remove unnecessary existence check File.exist?.

@haditariq haditariq changed the base branch from ketcher2-editor to main August 8, 2024 09:09
Copy link

github-actions bot commented Aug 8, 2024

LCOV of commit d43d75b during Continuous Integration #3390

Summary coverage rate:
  lines......: 65.2% (13637 of 20909 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@PiTrem PiTrem added this to the v1.10.0 milestone Aug 13, 2024
@PiTrem PiTrem requested a review from adambasha0 August 13, 2024 07:42
@haditariq haditariq changed the title User templates implementation with ketcher2 ketcher2 User templates Aug 15, 2024
Copy link

LCOV of commit 3cbdd3d during Continuous Integration #3482

Summary coverage rate:
  lines......: 65.2% (13638 of 20910 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@PiTrem PiTrem marked this pull request as draft August 16, 2024 13:43
@PiTrem PiTrem removed this from the v1.10.0 milestone Aug 20, 2024
@PiTrem
Copy link
Member

PiTrem commented Aug 20, 2024

made redundant by #2061

@PiTrem PiTrem closed this Aug 20, 2024
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.

2 participants