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

DatePicker: LocalDateTime conversion error when used inside ui:repeat #12234

Open
martingLab opened this issue Jul 2, 2024 · 10 comments · May be fixed by #12242
Open

DatePicker: LocalDateTime conversion error when used inside ui:repeat #12234

martingLab opened this issue Jul 2, 2024 · 10 comments · May be fixed by #12242
Assignees
Labels
🐞 defect Bug...Something isn't working
Milestone

Comments

@martingLab
Copy link

martingLab commented Jul 2, 2024

Describe the bug

When p:datePicker is used for LocalDateTime and is placed inside ui:repeat, then a strange conversion error is raised. This does not happen for LocalDate and LocalTime.

Reproducer

Here's the code to reproduce:

xhtml:

 <h:form>
                 <p:messages closable="true"/>  <br/>
                 <ui:repeat value="#{datePickerBacking.testStrList}" var="rep" >
                     <h:panelGroup id="pgDateTime" >
                         <p:outputLabel value="#{rep}" for="dateTimeField" class="Fs12" style="padding-left: 5px;"/> <br/>
                         <p:datePicker id="dateTimeField" value="#{datePickerBacking.dateTime}" />
                         <p:commandButton value="search" actionListener="#{datePickerBacking.doSomething()}" process="`@form`" update="`@form`" /> <br/>
                     </h:panelGroup>
                 </ui:repeat>
             </h:form>

Backing bean:

 @ViewScoped
 @Named
 public class DatePickerBacking implements Serializable {
     private List<String> testStrList;
     private LocalDateTime dateTime;
     
     public DatePickerBacking() {
         testStrList = List.of("TestLabel 1", "TestLabel 2");
     }
     
     public void doSomething(){
         // do something
     }
 
     public LocalDateTime getDateTime() {
         return dateTime;
     }
 
     public void setDateTime(LocalDateTime dateTime) {
         this.dateTime = dateTime;
     }
 
     public List<String> getTestStrList() {
         return testStrList;
     }
 
     public void setTestStrList(List<String> testStrList) {
         this.testStrList = testStrList;
     }

Steps to trigger error:

  1. Select date and time in one of the two input fields
  2. Click 'search' button

Expected behavior

No response

PrimeFaces edition

Community

PrimeFaces version

13.0.10

Theme

No response

JSF implementation

Mojarra

JSF version

4.0

Java version

17

Browser(s)

No response

@martingLab martingLab added ‼️ needs-triage Issue needs triaging 🐞 defect Bug...Something isn't working labels Jul 2, 2024
@melloware
Copy link
Member

melloware commented Jul 2, 2024

Reproducer attached:
pf-12234.zip

Tested with 13.0.10 and 14.0.2 its the same issue so its not new whatever it is.

@melloware melloware removed the ‼️ needs-triage Issue needs triaging label Jul 2, 2024
@melloware melloware changed the title p:datePicker: LocalDateTime conversion error when used inside ui:repeat DatePicker: LocalDateTime conversion error when used inside ui:repeat Jul 2, 2024
@melloware
Copy link
Member

melloware commented Jul 2, 2024

Might be a Mojarra bug it works in MyFaces.

mvn clean jetty:run -Pmojarra40 FAILS

mvn clean jetty:run -Pmyfaces40 WORKS

@melloware
Copy link
Member

Actually it doesn't Myfaces doesn't throw an error but sets all dates to NOW.

@melloware
Copy link
Member

Actually no it does work in MyFaces. It looks like its setting the second value only because you are using the same variable for both dates. Definitely looks like a Mojarra issue.

Reproducer:
pf-12234.zip

@martingLab
Copy link
Author

I tested it with Mojarra and without Primefaces, so I replaced datePicker with:

<h:inputText value="#{testDateTimeBacking.dateTimeValues[repStatus.index]}" >
<f:convertDateTime type="localDateTime" pattern="dd.MM.yyyy HH:mm" />
</h:inputText>

It works. I assume it's a Primefaces bug. When looking at the error message you see that it's a conversion error when trying to convert to Date. This does not happen in plain Faces / Mojarra as you define the data type via the type="localDateTime" attribute of f:convertDateTime. Primefaces apparently does not recognize that a LocalDateTime object is used - and you cannot explicitly define the data type in p:datePicker.

As a workaround I now defined a custom converter and assigned it to p:datePicker via the converter attribute - that works.

@melloware
Copy link
Member

That is a good workaround but its strange that it works fine in MyFaces. So it might be related to the ui:repeat ?

@martingLab
Copy link
Author

Yes agree, strange that it works in MyFaces. But on the other hand it works in plain Faces / Mojarra within ui:repeat. So it's hard to raise an issue at the Mojarra project.
I think it must have something to do with the logic to implicitly identify the relevant DateTime data type in Primefaces / p:datePicker.

melloware added a commit to melloware/primefaces that referenced this issue Jul 3, 2024
@melloware
Copy link
Member

OK debugged it. You could also have fixed it by setting showTime="true" on the DatePicker. I am submitting a PR.

@tandraschko
Copy link
Member

did you try to debug, why are value type recognition is not working?

@melloware
Copy link
Member

melloware commented Jul 5, 2024

ValueType recognition is working fine it always detects it as a LocalDateTime correctly.

Yes I debugged and the issue is here:

protected Temporal convertToJava8DateTimeAPI(FacesContext context, UICalendar calendar, Class<?> type, String submittedValue) {
        if (type == LocalDate.class || type == YearMonth.class || (type == LocalDateTime.class && !calendar.hasTime())) {

the value of calendar.hasTime() is false inside the Mojarra UI Repeat . Has Time for DatePicker is actually this.

@Override
    public boolean hasTime() {
        return this.isShowTime() || this.isTimeOnly();
    }

The value of this.isShowTime() is false when it should be true.

So for @martingLab that is why i said if on his DatePicker he set showTime="true" it would work as the value would always evaluate to true.

But in our DatePicker we try and dynamically determine if showTime="true"

 if (datePicker.isShowTimeWithoutDefault() == null) {
            Class<?> type = datePicker.getValueType();

            if (type != null) {
                datePicker.setShowTime(LocalDateTime.class.isAssignableFrom(type));
            }
            else {
                datePicker.setShowTime(CalendarUtils.hasTime(pattern));
            }
        }

That is done on the EncodeEnd. But for some reason in the ui:repeat of mojarra this value of showtime is still false even though it was set to true in EncodeEnd.

So the fix was to just also check this code in decode to make sure the values are set correctly upon decode if thre was no value for showTime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 defect Bug...Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants