strictdoc-project / strictdoc

Software for technical documentation and requirements management.
https://strictdoc.readthedocs.io/en/stable/
Other
157 stars 26 forks source link

UI: Cannot save element with SingleChoice field #1900

Open childen opened 5 months ago

childen commented 5 months ago

Steps to reproduce:

  1. Manually create grammar element using SingleChoice. E.g.
    - TAG: WITH_ENUM
    FIELDS:
    - TITLE: UID
    TYPE: String
    REQUIRED: False
    - TITLE: TITLE
    TYPE: String
    REQUIRED: False
    - TITLE: STATEMENT
    TYPE: String
    REQUIRED: False
    - TITLE: STATUS
    TYPE: SingleChoice(Draft, Ready, InReview)
    REQUIRED: False
    RELATIONS:
    - TYPE: Parent
  2. Start server
  3. Add new element of type using the SingleChoice field
  4. Enter valid values in all fields and click "Save"

For me the edit window stays open and no error is displayed. Additionally the backend prints the following error message: NotImplementedError: Field STATUS must be a single-line field.

Backtrace:

INFO:     127.0.0.1:44636 - "POST /actions/document/create_requirement HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
  + Exception Group Traceback (most recent call last):
  |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_utils.py", line 87, in collapse_excgroups
  |     yield
  |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 190, in __call__
  |     async with anyio.create_task_group() as task_group:
  |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 680, in __aexit__
  |     raise BaseExceptionGroup(
  | exceptiongroup.ExceptionGroup: unhandled errors in a TaskGroup (1 sub-exception)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/uvicorn/protocols/http/httptools_impl.py", line 399, in run_asgi
    |     result = await app(  # type: ignore[func-returns-value]
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 70, in __call__
    |     return await self.app(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/fastapi/applications.py", line 1054, in __call__
    |     await super().__call__(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/applications.py", line 123, in __call__
    |     await self.middleware_stack(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/errors.py", line 186, in __call__
    |     raise exc
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/errors.py", line 164, in __call__
    |     await self.app(scope, receive, _send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/cors.py", line 93, in __call__
    |     await self.simple_response(scope, receive, send, request_headers=headers)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/cors.py", line 148, in simple_response
    |     await self.app(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 189, in __call__
    |     with collapse_excgroups():
    |   File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
    |     self.gen.throw(typ, value, traceback)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_utils.py", line 93, in collapse_excgroups
    |     raise exc
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 191, in __call__
    |     response = await self.dispatch_func(request, call_next)
    |   File "[...]/strictdoc/./strictdoc/server/app.py", line 34, in add_process_time_header
    |     response = await call_next(request)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 165, in call_next
    |     raise app_exc
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 151, in coro
    |     await self.app(scope, receive_or_disconnect, send_no_error)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 65, in __call__
    |     await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    |     raise exc
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    |     await app(scope, receive, sender)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 756, in __call__
    |     await self.middleware_stack(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 776, in app
    |     await route.handle(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 297, in handle
    |     await self.app(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 77, in app
    |     await wrap_app_handling_exceptions(app, request)(scope, receive, send)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    |     raise exc
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    |     await app(scope, receive, sender)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 72, in app
    |     response = await func(request)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/fastapi/routing.py", line 278, in app
    |     raw_response = await run_endpoint_function(
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/fastapi/routing.py", line 191, in run_endpoint_function
    |     return await dependant.call(**values)
    |   File "[...]/strictdoc/./strictdoc/server/routers/main_router.py", line 960, in create_requirement
    |     html_generator.export_single_document_with_performance(
    |   File "[...]/strictdoc/./strictdoc/export/html/html_generator.py", line 264, in export_single_document_with_performance
    |     self.export_single_document(
    |   File "[...]/strictdoc/./strictdoc/export/html/html_generator.py", line 304, in export_single_document
    |     document_content = DocumentHTMLGenerator.export(
    |   File "[...]/strictdoc/./strictdoc/export/html/generators/document.py", line 32, in export
    |     return view_object.render_screen(html_templates.jinja_environment())
    |   File "[...]/strictdoc/./strictdoc/export/html/generators/view_objects/document_screen_view_object.py", line 94, in render_screen
    |     return template.render(view_object=self)
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/jinja2/environment.py", line 1304, in render
    |     self.environment.handle_exception()
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/jinja2/environment.py", line 939, in handle_exception
    |     raise rewrite_traceback_stack(source=source)
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_98f5f2d7961ff4e38c81cde238c0cb7961c9a6af.py", line 74, in top-level template code
    |     pass
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_e87dd9dec424f6fe5f73e2d63b714bda87e0c18e.py", line 69, in top-level template code
    |     yield '\n\n  <title>'
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_98f5f2d7961ff4e38c81cde238c0cb7961c9a6af.py", line 90, in block 'main_content'
    |     yield '"></script>\n  <script type="module" src="'
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_aebdf9dbb548b52a0ed7d3c534d9db86dd34096b.py", line 2, in top-level template code
    |     name = 'screens/document/document/main.jinja'
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_98eee7c33078735bf6129cb72fc27f293cb3d3d2.py", line 16, in top-level template code
    |     for l_1_node in context.call(environment.getattr((undefined(name='view_object') if l_0_view_object is missing else l_0_view_object), 'document_content_iterator')):
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_fb732dc882ed137ab4019fcf7dea46c0e0456c28.py", line 3, in top-level template code
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_c20ee41a2e12a0bac6bdc6ec324c0b80c88e234a.py", line 62, in top-level template code
    |     template = environment.get_template('components/anchor/index.jinja', 'components/node/index.jinja')
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_fb732dc882ed137ab4019fcf7dea46c0e0456c28.py", line 5, in block 'sdoc_entity'
    |     resolve = context.resolve_or_missing
    |   File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_5d32b8d2ab2af452704dfc83500b5de6e4db577e.py", line 16, in top-level template code
    |     @internalcode
    |   File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/jinja2/environment.py", line 487, in getattr
    |     return getattr(obj, attribute)
    |   File "[...]/strictdoc/./strictdoc/backend/sdoc/models/node.py", line 210, in reserved_status
    |     return self._get_cached_field(
    |   File "[...]/strictdoc/./strictdoc/backend/sdoc/models/node.py", line 534, in _get_cached_field
    |     raise NotImplementedError(
    | NotImplementedError: Field STATUS must be a single-line field.
    +------------------------------------

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/uvicorn/protocols/http/httptools_impl.py", line 399, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 70, in __call__
    return await self.app(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/fastapi/applications.py", line 1054, in __call__
    await super().__call__(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/applications.py", line 123, in __call__
    await self.middleware_stack(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/errors.py", line 186, in __call__
    raise exc
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/errors.py", line 164, in __call__
    await self.app(scope, receive, _send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/cors.py", line 93, in __call__
    await self.simple_response(scope, receive, send, request_headers=headers)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/cors.py", line 148, in simple_response
    await self.app(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 189, in __call__
    with collapse_excgroups():
  File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
    self.gen.throw(typ, value, traceback)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_utils.py", line 93, in collapse_excgroups
    raise exc
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 191, in __call__
    response = await self.dispatch_func(request, call_next)
  File "[...]/strictdoc/./strictdoc/server/app.py", line 34, in add_process_time_header
    response = await call_next(request)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 165, in call_next
    raise app_exc
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/base.py", line 151, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 65, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 756, in __call__
    await self.middleware_stack(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 776, in app
    await route.handle(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 297, in handle
    await self.app(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 77, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/starlette/routing.py", line 72, in app
    response = await func(request)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/fastapi/routing.py", line 278, in app
    raw_response = await run_endpoint_function(
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/fastapi/routing.py", line 191, in run_endpoint_function
    return await dependant.call(**values)
  File "[...]/strictdoc/./strictdoc/server/routers/main_router.py", line 960, in create_requirement
    html_generator.export_single_document_with_performance(
  File "[...]/strictdoc/./strictdoc/export/html/html_generator.py", line 264, in export_single_document_with_performance
    self.export_single_document(
  File "[...]/strictdoc/./strictdoc/export/html/html_generator.py", line 304, in export_single_document
    document_content = DocumentHTMLGenerator.export(
  File "[...]/strictdoc/./strictdoc/export/html/generators/document.py", line 32, in export
    return view_object.render_screen(html_templates.jinja_environment())
  File "[...]/strictdoc/./strictdoc/export/html/generators/view_objects/document_screen_view_object.py", line 94, in render_screen
    return template.render(view_object=self)
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/jinja2/environment.py", line 1304, in render
    self.environment.handle_exception()
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/jinja2/environment.py", line 939, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_98f5f2d7961ff4e38c81cde238c0cb7961c9a6af.py", line 74, in top-level template code
    pass
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_e87dd9dec424f6fe5f73e2d63b714bda87e0c18e.py", line 69, in top-level template code
    yield '\n\n  <title>'
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_98f5f2d7961ff4e38c81cde238c0cb7961c9a6af.py", line 90, in block 'main_content'
    yield '"></script>\n  <script type="module" src="'
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_aebdf9dbb548b52a0ed7d3c534d9db86dd34096b.py", line 2, in top-level template code
    name = 'screens/document/document/main.jinja'
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_98eee7c33078735bf6129cb72fc27f293cb3d3d2.py", line 16, in top-level template code
    for l_1_node in context.call(environment.getattr((undefined(name='view_object') if l_0_view_object is missing else l_0_view_object), 'document_content_iterator')):
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_fb732dc882ed137ab4019fcf7dea46c0e0456c28.py", line 3, in top-level template code
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_c20ee41a2e12a0bac6bdc6ec324c0b80c88e234a.py", line 62, in top-level template code
    template = environment.get_template('components/anchor/index.jinja', 'components/node/index.jinja')
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_fb732dc882ed137ab4019fcf7dea46c0e0456c28.py", line 5, in block 'sdoc_entity'
    resolve = context.resolve_or_missing
  File "/tmp/strictdoc_cache/jinja/307b3ccc814f6952e26d5cad3b7468d4/tmpl_5d32b8d2ab2af452704dfc83500b5de6e4db577e.py", line 16, in top-level template code
    @internalcode
  File "[...]/strictdoc/build/tox/py310-development/lib/python3.10/site-packages/jinja2/environment.py", line 487, in getattr
    return getattr(obj, attribute)
  File "[...]/strictdoc/./strictdoc/backend/sdoc/models/node.py", line 210, in reserved_status
    return self._get_cached_field(
  File "[...]/strictdoc/./strictdoc/backend/sdoc/models/node.py", line 534, in _get_cached_field
    raise NotImplementedError(
NotImplementedError: Field STATUS must be a single-line field.
stanislaw commented 5 months ago

Hi @childen,

Thanks for reporting this! The Single and Multi Choice fields are almost implemented but not quite there yet. The progress is tracked here: https://github.com/strictdoc-project/strictdoc/issues/1076.

It looks like you are hitting the same issue as @haxtibal. StrictDoc has a not so well explained convention about single- and multi-line fields convention which he documented here just yesterday. Please check his documentation update, and let me know if slightly changing your grammar would fix the issue: https://github.com/strictdoc-project/strictdoc/pull/1898.

stanislaw commented 5 months ago

@haxtibal already suggested that the single/multiline fields could be placed below the statement field but be always treated as single-line fields. This could be a good option for overcoming this limitation.

childen commented 5 months ago

@stanislaw thank you for the clarification. Moving the field solves the issue in the dev environment.

Should we close this issue or keep it around to track one of the following solutions?

  1. Always treat Single/MultiChoice as single line fields (even if placed below statement field)
  2. Improve the error message. I.e. include the info, that Single/MultiChoice fields are only allowed to appread before the statement field.
stanislaw commented 5 months ago

Let's not close this issue. I will do the following:

Add a validation to the grammar validator in the CLI interface as well as in the UI interface. Both messages will prevent a grammar with this issue's example from being created.

This will satisfy the point 2 and will make the point 1 solved at the same time.

I am not happy with this convention but I don't know of a better way of handling this. Here is a fragment of my recent discussion with @haxtibal:

Hi @Tobias Deiminger . You are the first to hit and consider this seriously.

Currently, there is a gray area with a convention: every field which is before the content field is singleline (with your recent feedback, the content field is now STATEMENT/DESCRIPTION/CONTENT).

There are simple cases when StrictDoc must know whether it is dealing with a single or multi-line field. For example, you cannot receive a multiline value for the UID field, and this is where all the newline characters must be stripped. The same for TITLE. The cases like this made me to introduce this convention: if a field is before the content field, it is considered meta information and is assumed to be single-line. All fields after the content field are assumed to be multiline. An alternative I was considering for a long time was to make it explicit in the grammar that a field could be SinglelineString or MultilineString but I didn't get to implementing that yet and that would also enforce every field to have just one syntax: normal string or <<< >>>. I thought about making SingleChoice and MultipleChoice to be treated as single-line strings but didn't get to doing it 😦 The documentation could be doing a better job, indeed. How would you prefer to have it?

When I consider it from the strict-ness perspective, my clear preference would be to split the String type into Single/Multiline String types. But that would be another migration that has to preserve backward compatibility and still maintain the convention <= content field. So I guess the documentation has to explain this in any case as the first step.

Do you think you could contribute a sentence to the documentation, explaining this clearly just the way you yourself would like to have seen it when reading for the first time?

And then the response from @haxtibal:

My initial assumption was that one can define Single/MultipleChoice fields anywhere - and I did not think about whether they could be multiline at all. Regarding grammer I would probably find an explicit multiline string a bit confusing, just because I never saw such a distinction in any programming language. A string is a string, and if it contains \n it has multiple lines, that's it 🙂 Everything is actually ok for us as of now. Knowing that these fields must be defined before the content field is enough. I can contribute such a sentence.